Subject: [codec] Questions about FEC

From: Alfons Martin <[email-masked]>
Date: Wed, 17 Apr 2013 13:10:45 +0200

Sorry if you receive multiple copies.

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

-- 
Symonics GmbH

Sand 13

72076 Tübingen

Tel +49 7071 2970573

Fax +49 7071 5681309

Email: [email-masked]

Geschäftsführer/Presidents: Michael Haun, Dr. Christian Hoene, Patrick 
Schreiner

Sitz der Gesellschaft/Place of Business: Tübingen

Registereintrag/Commercial Register: Amtsgericht Stuttgart, HRB 739918

---

From: Jean-Marc Valin <[email-masked]>
Date: Wed, 24 Apr 2013 22:59:09 -0400

Hi,

I just checked in this fix in master:
https://git.xiph.org/?p=opus.git;a=commitdiff;h=1bf32bb;hp=f77410d
Let me know if you're still having issues.

Cheers,

	Jean-Marc