[FFmpeg-devel] [PATCH 1/5] bink: set audio stream codec_tag such that binkaudio decoder can identify bitstream version

[Ronald S. Bultje]

Hi,

On Sun, Feb 20, 2011 at 1:56 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On 20 Feb 2011, at 03:35, Peter Ross <pross at xvid.org> wrote:
>> On Sat, Feb 19, 2011 at 07:13:31PM -0700, Daniel Verkamp wrote:
>>> On Sat, Feb 19, 2011 at 6:24 PM, Peter Ross <pross at xvid.org> wrote:
>>>> ---
>>>> ?libavformat/bink.c | ? ?2 +-
>>>> ?1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/libavformat/bink.c b/libavformat/bink.c
>>>> index dfad6c2..875c1f8 100644
>>>> --- a/libavformat/bink.c
>>>> +++ b/libavformat/bink.c
>>>> @@ -134,7 +134,7 @@ static int read_header(AVFormatContext *s, AVFormatParameters *ap)
>>>> ? ? ? ? ? ? if (!ast)
>>>> ? ? ? ? ? ? ? ? return AVERROR(ENOMEM);
>>>> ? ? ? ? ? ? ast->codec->codec_type ?= AVMEDIA_TYPE_AUDIO;
>>>> - ? ? ? ? ? ?ast->codec->codec_tag ? = 0;
>>>> + ? ? ? ? ? ?ast->codec->codec_tag ? = vst->codec->codec_tag;
>>>> ? ? ? ? ? ? ast->codec->sample_rate = get_le16(pb);
>>>> ? ? ? ? ? ? av_set_pts_info(ast, 64, 1, ast->codec->sample_rate);
>>>> ? ? ? ? ? ? flags = get_le16(pb);
>>>> --
>>>> 1.7.1
>>>>
>>>
>>> Maybe this should get its own codec id? Either way, overall series
>>> looks good to me.
>>
>> Using tags to identify the different versions is in keeping with the
>> way its done for Bink video.
>
> It is still a bad idea, codec tag isn't necessarily preserved on stream copy for example (admittedly for now at least we cannot mix it into formats that do not use codec_tag anyway though).
> It still is giving people bad ideas for cases where this is an issue though.

I am going to queue this patchset anyway, but I do agree with Reimar
that this is a little bit of a problem, even if codecs aren't
completely different, differentiating on codec_tag alone is asking for
trouble in the long run.

Ronald