[FFmpeg-devel] [PATCH 1/2] avformat/riff: pass an AVFormatContext explicitly to ff_get_wav_header

[FFmpeg-devel] [PATCH 1/2] avformat/riff: pass an AVFormatContext explicitly to ff_get_wav_header

[Marton Balint]

Will be useful later.
---
 libavformat/riff.h    |  2 +-
 libavformat/riffdec.c | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/libavformat/riff.h b/libavformat/riff.h
index a93eadfeca..719fd7a17d 100644
--- a/libavformat/riff.h
+++ b/libavformat/riff.h
@@ -67,7 +67,7 @@ void ff_put_bmp_header(AVIOContext *pb, AVCodecParameters *par, int for_asf, int
 int ff_put_wav_header(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *par, int flags);
 
 enum AVCodecID ff_wav_codec_get_id(unsigned int tag, int bps);
-int ff_get_wav_header(void *logctx, AVIOContext *pb, AVCodecParameters *par,
+int ff_get_wav_header(AVFormatContext *ctx, AVIOContext *pb, AVCodecParameters *par,
                       int size, int big_endian);
 
 extern const AVCodecTag ff_codec_bmp_tags[]; // exposed through avformat_get_riff_video_tags()
diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c
index 0fe4e02b7b..6cc912133c 100644
--- a/libavformat/riffdec.c
+++ b/libavformat/riffdec.c
@@ -58,7 +58,7 @@ enum AVCodecID ff_codec_guid_get_id(const AVCodecGuid *guids, ff_asf_guid guid)
  * an openended structure.
  */
 
-static void parse_waveformatex(void *logctx, AVIOContext *pb, AVCodecParameters *par)
+static void parse_waveformatex(AVFormatContext *ctx, AVIOContext *pb, AVCodecParameters *par)
 {
     ff_asf_guid subformat;
     int bps;
@@ -84,21 +84,21 @@ static void parse_waveformatex(void *logctx, AVIOContext *pb, AVCodecParameters
     } else {
         par->codec_id = ff_codec_guid_get_id(ff_codec_wav_guids, subformat);
         if (!par->codec_id)
-            av_log(logctx, AV_LOG_WARNING,
+            av_log(ctx, AV_LOG_WARNING,
                    "unknown subformat:"FF_PRI_GUID"\n",
                    FF_ARG_GUID(subformat));
     }
 }
 
 /* "big_endian" values are needed for RIFX file format */
-int ff_get_wav_header(void *logctx, AVIOContext *pb,
+int ff_get_wav_header(AVFormatContext *ctx, AVIOContext *pb,
                       AVCodecParameters *par, int size, int big_endian)
 {
     int id, channels = 0, ret;
     uint64_t bitrate = 0;
 
     if (size < 14) {
-        avpriv_request_sample(logctx, "wav header size < 14");
+        avpriv_request_sample(ctx, "wav header size < 14");
         return AVERROR_INVALIDDATA;
     }
 
@@ -139,18 +139,18 @@ int ff_get_wav_header(void *logctx, AVIOContext *pb,
     if (size >= 18 && id != 0x0165) {  /* We're obviously dealing with WAVEFORMATEX */
         int cbSize = avio_rl16(pb); /* cbSize */
         if (big_endian) {
-            avpriv_report_missing_feature(logctx, "WAVEFORMATEX support for RIFX files");
+            avpriv_report_missing_feature(ctx, "WAVEFORMATEX support for RIFX files");
             return AVERROR_PATCHWELCOME;
         }
         size  -= 18;
         cbSize = FFMIN(size, cbSize);
         if (cbSize >= 22 && id == 0xfffe) { /* WAVEFORMATEXTENSIBLE */
-            parse_waveformatex(logctx, pb, par);
+            parse_waveformatex(ctx, pb, par);
             cbSize -= 22;
             size   -= 22;
         }
         if (cbSize > 0) {
-            ret = ff_get_extradata(logctx, par, pb, cbSize);
+            ret = ff_get_extradata(ctx, par, pb, cbSize);
             if (ret < 0)
                 return ret;
             size -= cbSize;
@@ -163,7 +163,7 @@ int ff_get_wav_header(void *logctx, AVIOContext *pb,
         int nb_streams, i;
 
         size -= 4;
-        ret = ff_get_extradata(logctx, par, pb, size);
+        ret = ff_get_extradata(ctx, par, pb, size);
         if (ret < 0)
             return ret;
         nb_streams         = AV_RL16(par->extradata + 4);
@@ -179,7 +179,7 @@ int ff_get_wav_header(void *logctx, AVIOContext *pb,
     par->bit_rate = bitrate;
 
     if (par->sample_rate <= 0) {
-        av_log(logctx, AV_LOG_ERROR,
+        av_log(ctx, AV_LOG_ERROR,
                "Invalid sample rate: %d\n", par->sample_rate);
         return AVERROR_INVALIDDATA;
     }
-- 
2.35.3

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Marton Balint]

- Only parse the defined masks in dwChannelMask, unless strict_std_compliance
  is less than normal. This matches with the behaviour of the wav muxer.
- Ignore additional bits in dwChannelMasks as the MS documentation suggests [1]
- Assume UNKNOWN channels for missing bits as the MS documentation suggests [1]

[1] https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c
index 6cc912133c..0c7b2c6bdb 100644
--- a/libavformat/riffdec.c
+++ b/libavformat/riffdec.c
@@ -58,7 +58,7 @@ enum AVCodecID ff_codec_guid_get_id(const AVCodecGuid *guids, ff_asf_guid guid)
  * an openended structure.
  */
 
-static void parse_waveformatex(AVFormatContext *ctx, AVIOContext *pb, AVCodecParameters *par)
+static int parse_waveformatex(AVFormatContext *ctx, AVIOContext *pb, AVCodecParameters *par, int channels)
 {
     ff_asf_guid subformat;
     int bps;
@@ -69,7 +69,26 @@ static void parse_waveformatex(AVFormatContext *ctx, AVIOContext *pb, AVCodecPar
         par->bits_per_coded_sample = bps;
 
     mask = avio_rl32(pb); /* dwChannelMask */
-    av_channel_layout_from_mask(&par->ch_layout, mask);
+    if (ctx->strict_std_compliance >= FF_COMPLIANCE_NORMAL)
+        mask &= 0x3ffff;
+
+    /* According to MS docs, most significant bits should be ignored if mask
+     * has more bits than the number of channels */
+    while (av_popcount64(mask) > channels)
+        mask &= ~(1LL << av_log2(mask));
+
+    /* If the mask has less bits than channels, then we assign the remaining
+     * channels as UNKNOWN. */
+    if (mask && av_popcount64(mask) < channels) {
+        int ret = av_channel_layout_custom_init(&par->ch_layout, channels);
+        if (ret < 0)
+            return ret;
+        for (int i = 0, idx = 0; mask; i++, mask >>= 1)
+            if (mask & 1)
+                par->ch_layout.u.map[idx++].id = i;
+    } else {
+        av_channel_layout_from_mask(&par->ch_layout, mask);
+    }
 
     ff_get_guid(pb, &subformat);
     if (!memcmp(subformat + 4,
@@ -88,6 +107,7 @@ static void parse_waveformatex(AVFormatContext *ctx, AVIOContext *pb, AVCodecPar
                    "unknown subformat:"FF_PRI_GUID"\n",
                    FF_ARG_GUID(subformat));
     }
+    return 0;
 }
 
 /* "big_endian" values are needed for RIFX file format */
@@ -145,7 +165,9 @@ int ff_get_wav_header(AVFormatContext *ctx, AVIOContext *pb,
         size  -= 18;
         cbSize = FFMIN(size, cbSize);
         if (cbSize >= 22 && id == 0xfffe) { /* WAVEFORMATEXTENSIBLE */
-            parse_waveformatex(ctx, pb, par);
+            ret = parse_waveformatex(ctx, pb, par, channels);
+            if (ret < 0)
+                return ret;
             cbSize -= 22;
             size   -= 22;
         }
-- 
2.35.3

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 998 bytes --]

On Sun, Mar 17, 2024 at 08:57:29PM +0100, Marton Balint wrote:
> - Only parse the defined masks in dwChannelMask, unless strict_std_compliance
>   is less than normal. This matches with the behaviour of the wav muxer.
> - Ignore additional bits in dwChannelMasks as the MS documentation suggests [1]
> - Assume UNKNOWN channels for missing bits as the MS documentation suggests [1]
> 
> [1] https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)

breaks:
./ffmpeg  -i ~/tickets/2859/5.1plusdownmix.wav -ac 2 -t 100 -bitexact -c:a aac -y /tmp/2859-frenchspeack-nolibfaac.mp4

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Marton Balint]

On Tue, 19 Mar 2024, Michael Niedermayer wrote:

> On Sun, Mar 17, 2024 at 08:57:29PM +0100, Marton Balint wrote:
>> - Only parse the defined masks in dwChannelMask, unless strict_std_compliance
>>   is less than normal. This matches with the behaviour of the wav muxer.
>> - Ignore additional bits in dwChannelMasks as the MS documentation suggests [1]
>> - Assume UNKNOWN channels for missing bits as the MS documentation suggests [1]
>>
>> [1] https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
>>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> breaks:
> ./ffmpeg  -i ~/tickets/2859/5.1plusdownmix.wav -ac 2 -t 100 -bitexact -c:a aac -y /tmp/2859-frenchspeack-nolibfaac.mp4

After the patch this is file will need -strict unofficial to work, since 
the downmix channels are not officially recognized in the dwChannelMask.

Regards,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Tobias Rapp]

On 19/03/2024 20:14, Marton Balint wrote:

>
>
> On Tue, 19 Mar 2024, Michael Niedermayer wrote:
>
>> On Sun, Mar 17, 2024 at 08:57:29PM +0100, Marton Balint wrote:
>>> - Only parse the defined masks in dwChannelMask, unless 
>>> strict_std_compliance
>>>   is less than normal. This matches with the behaviour of the wav 
>>> muxer.
>>> - Ignore additional bits in dwChannelMasks as the MS documentation 
>>> suggests [1]
>>> - Assume UNKNOWN channels for missing bits as the MS documentation 
>>> suggests [1]
>>>
>>> [1] 
>>> https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
>>>  1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> breaks:
>> ./ffmpeg  -i ~/tickets/2859/5.1plusdownmix.wav -ac 2 -t 100 -bitexact 
>> -c:a aac -y /tmp/2859-frenchspeack-nolibfaac.mp4
>
> After the patch this is file will need -strict unofficial to work, 
> since the downmix channels are not officially recognized in the 
> dwChannelMask.
>
I think downmix channels are part of the RF64 specification, see EBU 
Tech 3306 section 3.1:

"""
3.1 Enhancement for a PCM stereo down mix

No PCM stereo signal is included in the basic Wave Format Extensible.

To include a stereo channel the following is added:

#define SPEAKER_STEREO_LEFT    0x20000000
#define SPEAKER_STEREO_RIGHT    0x40000000
"""

Regards, Tobias

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Marton Balint]

On Thu, 21 Mar 2024, Tobias Rapp wrote:

> On 19/03/2024 20:14, Marton Balint wrote:
>
>> 
>>
>>  On Tue, 19 Mar 2024, Michael Niedermayer wrote:
>>
>>>  On Sun, Mar 17, 2024 at 08:57:29PM +0100, Marton Balint wrote:
>>>>  - Only parse the defined masks in dwChannelMask, unless
>>>>  strict_std_compliance
>>>>    is less than normal. This matches with the behaviour of the wav muxer.
>>>>  - Ignore additional bits in dwChannelMasks as the MS documentation
>>>>  suggests [1]
>>>>  - Assume UNKNOWN channels for missing bits as the MS documentation
>>>>  suggests [1]
>>>>
>>>>  [1]
>>>>  https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask
>>>>
>>>>  Signed-off-by: Marton Balint <cus@passwd.hu>
>>>>  ---
>>>>   libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
>>>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>>
>>>  breaks:
>>>  ./ffmpeg  -i ~/tickets/2859/5.1plusdownmix.wav -ac 2 -t 100 -bitexact
>>>  -c:a aac -y /tmp/2859-frenchspeack-nolibfaac.mp4
>>
>>  After the patch this is file will need -strict unofficial to work, since
>>  the downmix channels are not officially recognized in the dwChannelMask.
>> 
> I think downmix channels are part of the RF64 specification, see EBU Tech 
> 3306 section 3.1:
>
> """
> 3.1 Enhancement for a PCM stereo down mix
>
> No PCM stereo signal is included in the basic Wave Format Extensible.
>
> To include a stereo channel the following is added:
>
> #define SPEAKER_STEREO_LEFT    0x20000000
> #define SPEAKER_STEREO_RIGHT    0x40000000
> """
>

I was reluctant to add these, because the recommendation which superseded 
this, ITU BS.2088 does not mention these masks.

Nevertheless, you are right, these should be recognized to support 
historical RF64 files. Or we should not even make the distinction of RF64?

Regards,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/riffdec: follow the MS docs more strictly for setting wav channel layouts

[Tobias Rapp]

On 21/03/2024 10:40, Marton Balint wrote:

>
>
> On Thu, 21 Mar 2024, Tobias Rapp wrote:
>
>> On 19/03/2024 20:14, Marton Balint wrote:
>>
>>>
>>>
>>>  On Tue, 19 Mar 2024, Michael Niedermayer wrote:
>>>
>>>>  On Sun, Mar 17, 2024 at 08:57:29PM +0100, Marton Balint wrote:
>>>>>  - Only parse the defined masks in dwChannelMask, unless
>>>>>  strict_std_compliance
>>>>>    is less than normal. This matches with the behaviour of the wav 
>>>>> muxer.
>>>>>  - Ignore additional bits in dwChannelMasks as the MS documentation
>>>>>  suggests [1]
>>>>>  - Assume UNKNOWN channels for missing bits as the MS documentation
>>>>>  suggests [1]
>>>>>
>>>>>  [1]
>>>>>  https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653308(v=vs.85)#details-about-dwchannelmask 
>>>>>
>>>>>
>>>>>  Signed-off-by: Marton Balint <cus@passwd.hu>
>>>>>  ---
>>>>>   libavformat/riffdec.c | 28 +++++++++++++++++++++++++---
>>>>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>>>
>>>>  breaks:
>>>>  ./ffmpeg  -i ~/tickets/2859/5.1plusdownmix.wav -ac 2 -t 100 -bitexact
>>>>  -c:a aac -y /tmp/2859-frenchspeack-nolibfaac.mp4
>>>
>>>  After the patch this is file will need -strict unofficial to work, 
>>> since
>>>  the downmix channels are not officially recognized in the 
>>> dwChannelMask.
>>>
>> I think downmix channels are part of the RF64 specification, see EBU 
>> Tech 3306 section 3.1:
>>
>> """
>> 3.1 Enhancement for a PCM stereo down mix
>>
>> No PCM stereo signal is included in the basic Wave Format Extensible.
>>
>> To include a stereo channel the following is added:
>>
>> #define SPEAKER_STEREO_LEFT    0x20000000
>> #define SPEAKER_STEREO_RIGHT    0x40000000
>> """
>>
>
> I was reluctant to add these, because the recommendation which 
> superseded this, ITU BS.2088 does not mention these masks.
>
> Nevertheless, you are right, these should be recognized to support 
> historical RF64 files. Or we should not even make the distinction of 
> RF64?
>
WAV files smaller than 2/4GiB could start with the usual "RIFF" bytes 
but still contain these mask bits. From a quick glance at ITU BS.2088 it 
seems it could use the "chna" chunk to achieve a mapping like "5.1 + 
downmix".

Having said that, I have not stumbled over "X plus downmix" WAVE files 
in production yet. So no strong opinion from my side on whether this 
should work without "-strict unofficial", or not.

Regards, Tobias

_______________________________________________
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".