[FFmpeg-devel] [PATCH] Speex parser

[Justin Ruggles]

Baptiste Coudurier wrote:

> On 08/30/2009 06:26 PM, Justin Ruggles wrote:
>> Baptiste Coudurier wrote:
>>
>>> On 8/30/2009 5:50 PM, Justin Ruggles wrote:
>>>> Justin Ruggles wrote:
>>>>
>>>>> Justin Ruggles wrote:
>>>>>
>>>>>> Michael Niedermayer wrote:
>>>>>>
>>>>>>> On Sun, Aug 30, 2009 at 10:40:45AM -0400, Justin Ruggles wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Here is a Speex parser.  It does not split any frames, but only analyzes
>>>>>>>> them in order to set the proper AVCodecContext parameters.
>>>>>>> the decoder can do this, av_find_stream_info() should already create a
>>>>>>> decoder to fill these in when they are missing.
>>>>>> Why should it have to rely on the decoder...especially since we do not
>>>>>> have a native decoder?  So that one MUST compile in an external library
>>>>>> for stream copy to work properly.
>>>>> If there is no problem with packet duration being 0 or wrong, then I
>>>>> think stream copy could work without the parser or decoder.  I tried flv
>>>>> to ogg and it seemed to work since timestamps were just copied from one
>>>>> container to the other.  Packet duration was still wrong though, and I
>>>>> don't know if that causes other problems.
>>>> Ok, I think I figured out the solution to this part at least.  Speex
>>>> needs to be added to the list of codecs in has_codec_parameters() that
>>>> require frame_size to be non-zero, then the libspeex decoder should not
>>>> set frame_size at init when it does not have extradata since it could be
>>>> the wrong value.
>>> Or finally set sample_fmt to SAMPLE_FMT_NONE triggering decoding of the
>>> first frame.
>> That seems like a hack.  We always know the sample format at decoder
>> init, but we don't necessarily know the frame size.
> 
> This depends on codec. Codecs supporting different bit depth certainly 
> don't know the bit depth at init. Does it look like a hack ? I really 
> don't think so.


I thought you were just talking about Speex.  In general, yes I agree
with you that the decoder might not know it at init.

If you are suggesting that having SAMPLE_FMT_NONE as default would be a
preferable situation, then I agree with you.  But it seems wrong to me
to force av_find_stream_info() to always defer to the decoder to
complete all stream parameters.  In my opinion, the sample format output
by the decoder has nothing to do with the stream parameters as far as
the demuxer is concerned.

A grep of libavformat shows 4 uses of sample_fmt, not including the one
in has_codec_parameters().  3 look like incorrect uses.  One is just
questionable.  What do you think about fixing these and removing the
sample_fmt requirement from has_codec_parameters() and possibly from
libavformat completely?  Would it then be safe to make SAMPLE_FMT_NONE
the default instead of SAMPLE_FMT_S16?

-Justin