[FFmpeg-devel] [PATCH] Add a parser for DNET (byte-swapped AC3).

[Justin Ruggles]

On 03/02/2011 06:08 PM, Reimar D?ffinger wrote:

> On 3 Mar 2011, at 00:00, Reimar D?ffinger <Reimar.Doeffinger at gmx.de> wrote:
>> On 2 Mar 2011, at 23:43, Justin Ruggles <justin.ruggles at gmail.com> wrote:
>>> On 03/02/2011 05:18 PM, M?ns Rullg?rd wrote:
>>>
>>>> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>>>>
>>>>> On Wed, Mar 02, 2011 at 09:10:21PM +0000, M?ns Rullg?rd wrote:
>>>>>> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>>>>>>>> Then how did you intend to detect it as ac3 in the first place?
>>>>>>>
>>>>>>> By the fact that the RealMedia header says "DNET"?
>>>>>>
>>>>>> AC3 exists outside of realmedia file, you know...
>>>>>
>>>>> So what? Has anyone seen the byte-swapped variant outside a container?
>>>>> Either way libavformat has a better infrastructure to do this kind
>>>>> of format detection when it's hard to decide, a parser is not really
>>>>> such a great place for it.
>>>>
>>>> The parser can reasonably assume it is being fed AC3 data in some form
>>>> or other, so the difficulty of detection is irrelevant there.
>>>>
>>>>> Which, as said, is in addition to the fact whether there is a point
>>>>> at all of supporting byte-swapped raw AC3. It obviously exists
>>>>> in other containers, but already for that there seem to be no samples
>>>>> available, DNET is the only one actually common (and even that seems
>>>>> relative).
>>>>
>>>> AC3 in WAV is fairly common.  I'm sure we have a few samples of that.
>>>>
>>>> All else aside, calling it DNET is ridiculous.  It is called AC3
>>>> whatever container you wrap it in.  We don't have a CODEC_ID_ for every
>>>> tag used in some obscure container, we have one per codec.  Please keep
>>>> it that way.
>>>
>>>
>>> I agree. We can handle this in a different way.
>>>
>>> For example, I think it would be fine for the AC3 parser to detect byte
>>> order and then not accept switching byte order during parsing.  This
>>> would mitigate the false positive probability increase for all but the
>>> first frame, which is ok with me.
>>
>> Why would you want to do this? Making things difficult just for the heck of it or what?
>> There is _no_ AC3 decoder that even tries to do this.
>> Just to avoid a CODEC_ID? We do have one for each raw format, including differently swapped ones. And you would be ready to accept an error rate of 1:64000 just for that?

For the first frame, yes. It's not that much different than the current
probability.  When calculated over the whole stream, yes it's
significant, but for just 1 frame it's not.  The parser would have to be
fed a partial first frame or non-AC3 data for it to have even that
1:64000 chance of false positive.  For remaining frames it would be the
same as it is currently.

> And I forgot: stream copy would end up creating files that only FFmpeg can play unless we add a special hack.
> Contrary to the different DTS formats which are all really DTS, byte-swapped AC-3 is not considered AC-3 by any implementation I know of.


Dolby's decoder can handle byte-swapped AC3.  I would guess that other
Dolby-licensed decoders can as well.  I do see your point though that
treating byte-swapped AC3 as identical to AC3 could cause problems
during muxing when the byte order matters depending on the container.

Even if we do add a separate CODEC_ID for swapped AC3, the parser could
handle it in a similar way to AC3 vs EAC3 so we can support swapped AC3
in cases where it's not known by the demuxer (e.g. raw).  We could add
the new CODEC_ID to the list of supported codecs for ff_ac3_parser, then
detect byte swapped input and set codec_id accordingly.

So basically, I'm ok with the patch except that I think it can be more
closely integrated into the AC3 parser rather than adding a new parser.
 That could also be done later though.  And I don't like using the name
CODEC_ID_DNET.  CODEC_ID_AC3_LE might be a better name.

-Justin