[FFmpeg-devel] [PATCH] Ignore bad mapping in AAC (issue 1882)

[Alex Converse]

On Mon, May 31, 2010 at 10:11 AM, Cyril Russo
<stage.nexvision at laposte.net> wrote:
[snip]
>>>
>>> Do you mean get_che top part:
>>>
>>> ? ?if (ac->tag_che_map[type][elem_id]) {
>>> ? ? ? ?return ac->tag_che_map[type][elem_id];
>>> ? ?}
>>> is changed to (pseudo code):
>>> ? ?if (ac->tag_che_map[type][elem_id]) {
>>> ? ? ? ?if (ac->tags_mapped == tags_per_config[ac->m4ac.chan_config]) {
>>> ? ? ? ? ? ?return ac->tag_che_map[type][elem_id];
>>> ? ? ? ?} else {
>>> ? ? ? ? ? ?ac->tag_che_map[type][elem_id + 1] = ac->che[type][elem_id];
>>> ? ? ? ? ? ?ac->tags_mapped++;
>>> ? ? ? ? ? ?return ac->tag_che_map[type][elem_id + 1];
>>> ? ? ? ?}
>>> ? ?}
>>>
>>
>> I don't think this would work for your SCE[0] CPE[0] CPE[0] LFE[0]
>> case because then both CPEs would be mapped to elem_id + 1 = 0 + 1 =
>> 1.
>>
>> I rather mean to have an option in the context and avcodec.h exposed
>> to the user to toggle a completely different (and naive) mapping mode.
>> The mode would bypass the main code of get_che () and rather keep a
>> count of the number of syntax elements of a particular type that have
>> been encountered thus far. I think Alex is thinking the same but I
>> don't quite understand his (16*count + elem_id) comment on roundup.
>> ? Here is what I'm thinking:
>>
>> - have a flag for this mapping mode in the context and expose it to
>> the user through avcodec.h
>> - have syntax element count variables in the context
>> - map syntax elements using the following algorithm:
>> ? if syntax element has been mapped already, use the mapping
>> ? else map to [type][syntax element count] and increment syntax element
>> count
>>
>> This should then do the following for the SCE[0] CPE[0] CPE[0] LFE[0]
>> stream:
>>
>> Receive SCE[0], not yet mapped, sce_count is currently 0, map to
>> [TYPE_SCE][sce_count], sce_count++.
>> Receive CPE[0], not yet mapped, cpe_count is currently 0, map to
>> [TYPE_CPE][cpe_count], cpe_count++.
>> Receive CPE[0] (except this is the second CPE[0]), not yet mapped,
>> cpe_count is currently 1, map to [TYPE_CPE][cpe_count], cpe_count++.
>> Receive LFE[0], not yet mapped, lfe_count is currently 0, map to
>> [TYPE_LFE][lfe_count], lfe_count++.
>>
>> The difficult part of this code is deciding when something is not yet
>> mapped. I think it has to conduct the mapping based on merely the
>> syntax elements encountered in the first frame ignoring the element
>> ids and then consider them to be mapped.
>>
>
> Ok, then let's sum this up, we have to add a sce_count, cpe_count and
> lfe_count member to the context.
> Then, in codec init, set them to 0.
>
> If the codec's workaround_bugs contains a specific flag, then we bypass
> get_che actual code if and only if codec's frame_number is 0.
> In the bypassed version, we map as you said.
>

In general ac->output_configured should be relied on rather than frame number.

It seems like we should be able to auto detect this just fine. Instead
of keeping track of sce/cpe/lfe counts let's just track if we've seen
a specific elem_type/elem_id combo this frame.

> I propose this addition:
> In the code I've tried to fix (top of get_che), I change this:
>
> ? if (ac->tag_che_map[type][elem_id]) {
> ? ? ? return ac->tag_che_map[type][elem_id];
> ? }
> is changed to (pseudo code):
> ? if (ac->tag_che_map[type][elem_id]) {
> ? ? ? if (ac->tags_mapped == tags_per_config[ac->m4ac.chan_config]) {
> ? ? ? ? ? return ac->tag_che_map[type][elem_id];
> ? ? ? } else {
> ? ? ? ? ? // Log warning
> ? ? ? ? ? // Enable workaround_bugs in the context,
> ? ? ? ? ? // Count the used mapped element and increment sce/cpe/lfe count
> accordingly
> ? ? ? ? ? // Map the badding tagged element
> ? ? ? }
> ? }
>

With this approach you would always have to count, otherwise on
subsequent frames you would return the che mapped to the base elem_id,
no?

>> It's been a while since I looked at and thought about this code and
>> part of the spec, but I have a vague memory that we will have to
>> implement the workaround in other places in the decoder as well. That
>> is, I think there are other parts of the decoder that depend on the
>> element ids being correct. Maybe it would be better to note when
>> parsing the first frame if multiple elements with the same element id
>> are encountered and then fudge the element ids that are used from
>> there onward. Alex, what do you think?
>>
>
> Can you check this again ?
>

CCEs require correct target element tags. I don't know how to even
make sense of a CCE in a context where all tags are the same.

> Cheers,
> Cyril
>
>

What do people think of the attached approach?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dupe-channel.diff
Type: text/x-patch
Size: 1738 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100531/231580bc/attachment.bin>