Re: [codec] Questions about FEC

[Jean-Marc Valin <[e-mail]>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Thanks for reporting this. I'll need to have a closer look at that
code (which is relatively new and wasn't in the RFC) to see what's
going on.

	Jean-Marc

On 04/17/2013 05:45 AM, Alfons Martin wrote:
> Dear Jean-Marc,
> 
> I've got some problems with the FEC of Opus using the current
> stable release (1.02) from the repository. According to my
> understanding of FEC, the FEC information of the next packet is
> used to restore the former lost packet. But if I use the 
> "packet_size of the lost packet" to invoke FEC for it, as
> recomended in the API documentation, the function
> opus_decode_native uses PLC because of line 748 in file
> opus_decoder.c:
> 
> "if (frame_size < packet_frame_size || packet_mode ==
> MODE_CELT_ONLY || st->mode == MODE_CELT_ONLY){"
> 
> If i use "max_frame_size", like in the opus_demo invokation of 
> opus_decode (file opus_demo.c, line 714), FEC is used for the last 
> packet. But the recursive call inserts PLC for the other packets
> until "max_frame_size" is reached. This behaviour leads to a
> fragmented output-file.
> 
> If i use twice the "packet_size of the lost packet" to invoke 
> opus_decode, PLC and FEC still seem to be called for only 1 lost
> packet and the outputfile is fragmented again.
> 
> So i changed file opus_decoder.c line 748
> 
> if (frame_size <= packet_frame_size || packet_mode ==
> MODE_CELT_ONLY || st->mode == MODE_CELT_ONLY)
> 
> to
> 
> if (frame_size < packet_frame_size || packet_mode == MODE_CELT_ONLY
> || st->mode == MODE_CELT_ONLY)
> 
> But then
> 
> /* Otherwise, run the PLC on everything except the size for which
> we might have FEC */ duration_copy = st->last_packet_duration; ret
> = opus_decode_native(st, NULL, 0, pcm,
> frame_size-packet_frame_size, 0, 0, NULL);
> 
> is called anyway, and Opus never reaches the FEC part.
> 
> I then changed the code to the following and it worked, but i'm not
> sure if I missed sth.:
> 
> opus_decoder.c:738-764
> 
> if (decode_fec) { int duration_copy; int ret; /* If no FEC can be
> present, run the PLC (recursive call) */ if (frame_size <
> packet_frame_size || packet_mode == MODE_CELT_ONLY || st->mode ==
> MODE_CELT_ONLY){ return opus_decode_native(st, NULL, 0, pcm,
> frame_size, 0, 0, NULL); } if(frame_size == packet_frame_size &&
> packet_mode != MODE_CELT_ONLY && st->mode != MODE_CELT_ONLY){ /*
> Complete with FEC */ st->mode = packet_mode; st->bandwidth =
> packet_bandwidth; st->frame_size = packet_frame_size; 
> st->stream_channels = packet_stream_channels; ret =
> opus_decode_frame(st, data, size[0], 
> pcm+st->channels*(frame_size-packet_frame_size), packet_frame_size,
> 1); if (ret<0) return ret; }else{ /* Otherwise, run the PLC on
> everything except the size for which we might have FEC */ 
> duration_copy = st->last_packet_duration; ret =
> opus_decode_native(st, NULL, 0, pcm, frame_size-packet_frame_size,
> 0, 0, NULL); if (ret<0) { st->last_packet_duration =
> duration_copy; return ret; } } st->last_packet_duration =
> frame_size; return frame_size; }
> 
> What's your opinion on this?
> 
> Regards, Alfons
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRbuPpAAoJEJ6/8sItn9q9L3oH/jvFs5ECjE7hcGRB2L3h15P8
OwbpKduSC7JHq5GFBLDSgHvJ4VkkX+WvY+ZTlQR4TD+dFDjxrrC82Ab7xXKoHEi0
5H+GBN/mz3Usl7vow/wtAVX42Gk79dRDoKJYmWoFUWdttvCN7CPCUdnUqqctgiym
/KrjR48FzRLwXjjUVm1uO2IP9ITO4VGb4L/tZXyrCy8QlqgHoIXfDx2rBy15WB99
7fQPsNJfMw1bWEBrZ/1kaojH7PW0NyIcjVUDjryf15S9/RNPtSFA0trxbtbo3cZK
XCGDY/Q4dmZdqSI1DWGLCc2Lq4MMhJ8wuOh5bQjjc/+Cd9Bl3/a9jX7qTtKaJ78=
=4lMe
-----END PGP SIGNATURE-----