wrong mask in dxv

[projectsymphony]

for some reason 9b2802f0d3296059a61b0f323ee0fc86eba48bf5 was applied without any explanation

besides the fact that the decoder has been tested across several "real-world" files, the test is clearly wrong and could lead to awful results.

if (old_type & 0x20 || old_type & 0x2)

should definitely be

if ((old_type & 0x20) || (old_type & 0x2))

would it be possible to have one of the samples that made this change necessary?

[Carl Eugen Hoyos]

This ticket does not look valid (or correct) to me.

Concerning your question, sorry, my only answer is the same as when the new asf demuxer was presented: Where do you search for samples when (or after) implementing a new feature?

[projectsymphony]

Sorry if I wasn't clear with my ticket description.

The suggested change was only for clarity.

All samples I have (which come from "real-world") do not feature this header you found. It would be interesting if I could check it since it could contain further information in the headers that I haven't been able to reverse.

Would be able to provide one so that the decoder could improve further?

[Carl Eugen Hoyos]

I will point to the sample I used, please try to answer my question first, I believe it is really important to understand the difference between our projects / our attitude.

I am definitely thankful that you are looking at my patch, my original thought was that you just made a typo, it would be good if you could double-check.

[Carl Eugen Hoyos]

Replying to projectsymphony:

Sorry if I wasn't clear with my ticket description.

The suggested change was only for clarity.

Allow me to add that "only for clarity" and "clearly wrong" are not synonyms.

[Carl Eugen Hoyos]

I believe the sample in question can be found in ​https://samples.libav.org/V-codecs/DXDI/
If the binary decoder just does } else { then imo this is what libavcodec should do.

[projectsymphony]

Thanks for the sample. I was able to confirm that the sample is valid and it comes from the very first version of the codec (which supported only a single decoding mode, hence the absence of the else in the binary).

The check could be either removed entirely, or it could be set to test the encoder version exclusively. I think I would prefer the latter so here is attached a patch that does that.

[Carl Eugen Hoyos]

As said, I believe you should copy the binary decoders' behaviour, but you are the maintainer, so it is currently your decision.

[Hendrik]

Fixed in 234c7378ca1991d3389f25927e9e98184dccb4a1