[FFmpeg-devel] [PATCH] QCELP decoder

[Kenan Gillet]

Hi,
On Fri, Nov 14, 2008 at 2:27 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Nov 14, 2008 at 12:17:50PM -0800, Kenan Gillet wrote:
>>
>> On Nov 14, 2008, at 2:14 AM, Michael Niedermayer wrote:
> [...]
>> >
>> >
>> >> @@ -152,11 +537,140 @@
>> >>     return -1;
>> >> }
>> >>
>> >> +/*
>> >> + * Determine the framerate from the frame size and/or the first
>> >> byte of the frame.
>> >> + *
>> >> + * @param avctx the AV codec context
>> >> + * @param buf_size length of the buffer
>> >> + * @param buf the bufffer
>> >> + *
>> >> + * @return the framerate on success, RATE_UNKNOWN otherwise.
>> >> + */
>> >> +static int determine_framerate(AVCodecContext *avctx,
>> >> +                               const int buf_size,
>> >> +                               uint8_t **buf) {
>> >> +    qcelp_packet_rate framerate;
>> >> +
>> >> +    if ((framerate = buf_size2framerate(buf_size)) >= 0) {
>> >> +        if (framerate != **buf) {
>> >
>> > iam not sure but didnt you at some point reorder the enum?
>> > if so how can this code be correct before and afterwards?
>>
>>
>> I reorder the enum on the 09/07/2008, way before submitting my first
>> patch to
>>      RATE_FULL   = 0,
>>      RATE_HALF   = 1,
>>      RATE_QUARTER= 2,
>>      RATE_OCTAVE = 3,
>>      I_F_Q,          /*!< insufficient frame quality */
>>      BLANK,
>>      RATE_UNKNOWN
>> to
>>      SILENCE = 0,
>>      RATE_OCTAVE,
>>      RATE_QUARTER,
>>      RATE_HALF,
>>      RATE_FULL,
>>      I_F_Q,          /*!< insufficient frame quality */
>>      RATE_UNKNOWN
>> in order to reflect the rate byte in the QCELP frame.
>>
>> and I changed on the 10/27/2008 to
>>      RATE_UNKNOWN = -2,
>>      I_F_Q,             /*!< insufficient frame quality */
>>      SILENCE,
>>      RATE_OCTAVE,
>>      RATE_QUARTER,
>>      RATE_HALF,
>>      RATE_FULL
>> when you asked me to change the
>> switch (framerate)
>>    case RATE_FULL:
>>    case RATE_QUARTER:
>>    case RATE_OCTAVE:
>> }
>> to (framerate >= RATE_QUARTER)
>
> that doesnt awnser
> how the changing enum interacts with
> if(framerate != [some byte from the bitstream])
>

basically, the first byte of the frame corresponds to the rate in the enum.
the first byte can be
0 => SILENCE
1 => RATE_OCTAVE
2 => RATE_QUARTER
3 => RATE_HALF
4 => RATE_FULL

if it is not one of those, then we should have an I_F_Q.
The SoC code determined the framerate by looking at the bufsize
and then had a warning if the framerate byte (1st byte of the frame)
differed.
I suppose it was to handle frame which would not contain this first byte.
I have not seen any files with such a feature though, and neither
Reynaldo [1].

We could simplify to just looking at the framerate and checking
that the buffer contains enough data for the corresponding rate.

what do you think?

Renaldo, any thought on this one?


>
> [...]
>
>> attached the round 11:
>
> OutOfMissingAttachmentJokesException
>

lol

it is very bizarre because the email in my sent box has the attachment,
but it is seems to have been scrapped out somewhere along the way :(
I'll be using the web interface of gmail from now on.

hopefully the round 11 will be attached this time

[1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-November/055635.html
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: qcelp-round11-decoder.patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081114/5840e31d/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: qcelp-round11-doc-glue.patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081114/5840e31d/attachment-0001.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: qcelp-round11-lsp.patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081114/5840e31d/attachment-0002.txt>