Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

From: Nicolas Gaullier <nicolas.gaullier@cji.paris>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution
Date: Fri, 17 Feb 2023 09:44:17 +0000
Message-ID: <MR1P264MB2483BD964F0097624092701C9BA19@MR1P264MB2483.FRAP264.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <c7c8eb5e1bab031d6680a9087bc9e0a2384053cd.camel@haerdin.se>

>> @@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void *avc,
>>              avpriv_report_missing_feature(avc, "Data type %#x in 
>> SMPTE 337M", data_type & 0x1F);
>>          return AVERROR_PATCHWELCOME;
>>      }
>> +    if (container_word_bits &&
>> +        !(container_word_bits == 16 && word_bits == 16) &&
>> +        !(container_word_bits == 24 && word_bits == 20) &&
>
>I presume 20/24 is intentional. Does WAV not support signalling 20-bit?
>
>The rest looks OK enough
>
>/Tomas

You are true, I meant "container_word_bits" as "block_size" rather than "valid bits per sample" and
I think this should be clarified is the latter integration code in "wavdec: s337m support" patch where
I use par->bits_per_coded_sample...
But I should have rather coded "AV_CODEC_ID_PCM_S16LE ? 16 : 24"
This is in case a wav file would be detected as 20 bits in a 24 bits container (I don't think it is supported yet, but it could as bitspersample and validbitpersample are two different fields in WAVE_FORMAT_EXTENSIBLE).
Are you ok with this ?

Nicolas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".