[FFmpeg-devel] [PATCH] QCELP decoder

[Reynaldo H. Verdejo Pinochet]

Hello

Reynaldo H. Verdejo Pinochet wrote:
> 
> and then qcelp_bits_per_rate was exactly what its name would made
> you think it was. Can you remind me why did you change that enum's
> order? If that change is avoided this one would not be needed.

I searched the thread and discovered you had changed the enum
quite a few times to account for troubles with the original
change, and that original change was totally your idea...

 > 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.

I think that change was the wrong one. Can you please make
sure its really needed? I personally don't think so.

 > 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)
 >
 > After sending the patch round 10, I also added a check to make sure
 > the buffer
 > contains enough data for the the frame to be decoded without reading
 > garbage.

I dont think that change is needed neither as that should be
guaranteed by your demuxer - parser(?) chain.

 > Futhermore, I am dropping RATE_UNKNOWN and replace it by I_F_Q
 >   since the specification says at TIA/EIA/IS-733 2.4.8.7.1:
 > "If the received packet type cannot be satisfactorily determined, the
 > multiplex sublayer
 > informs the receiving speech codec of an erasure (see 2.3.2.2)."
 >
 > attached the round 11:

The idea of had a RATE_UNKNOWN was to be able to reflect an state.
Nothing wrong with you geting rid of it though but then again I'm
not sure you're gaining anything out of it.

Bests
--
Reynalo