[FFmpeg-devel] [PATCH 3/9] avformat/s337m: Consider container bit resolution

[Tomas Härdin]

mån 2020-01-13 klockan 14:02 +0000 skrev Gaullier Nicolas:
> > > +    if (container_word_bits && (container_word_bits+7)/8 != 
> > > + (word_bits+7)/8) {
> > 
> > Can it happen that word_bits is anything but 16 or 24 with a valid
> > stream? If not then I'd check container_word_bits == word_bits &&
> > (word_bits == 16 || word_bits == 24) or so
> word_bits may be 20, and in that case container_word_bits must be 24
> (this is the case in my fate test), so I think this is correct.

I think explicit checks for these three cases would be better instead
of rounding up. I wouldn't be surprised if there are files out there
that claim to be 18-bit or whatever..

> > > +    while ((container_word_bits == 24 || !IS_16LE_MARKER(state))
> > > +        && (container_word_bits == 16 || !IS_20LE_MARKER(state)
> > > && 
> > > + !IS_24LE_MARKER(state))) {
> > 
> > I'd rewrite this as while ((bits == 24 && (20LE || 24LE)) || (bits
> > ==20 && 16LE)), more readable
> container_word_bits may be 0 for autodetect, this results in this
> expression...
> I agree it is not that great for readability, but doing otherwise
> would require some additions and macros duplications, for example:
> while ( !(!bits && LE) || !(bits == 24 && (20LE || 24LE)) || !(bits
> ==16 && 16LE))
> Sounds heavy, not sure this is really better ?

Hrm, not sure tbh

/Tomas