diff mbox

[FFmpeg-devel] lavf/riffenc: Always write unexpected channel_mask

Message ID 201609261239.39069.cehoyos@ag.or.at
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Sept. 26, 2016, 10:39 a.m. UTC
Hi!

Attached patch allows to write arbitrary (mono) channel_masks 
even for 16bit 48kHz pcm audio.

Please comment, Carl Eugen
From bbbbde791edddeb16aadaa28f7ab53f1f60a9874 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Mon, 26 Sep 2016 12:36:54 +0200
Subject: [PATCH] lavf/riffenc: Always write unexpected channel_mask.

---
 libavformat/riffenc.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Tobias Rapp Sept. 27, 2016, 6:49 a.m. UTC | #1
On 26.09.2016 12:39, Carl Eugen Hoyos wrote:
> Hi!
>
> Attached patch allows to write arbitrary (mono) channel_masks
> even for 16bit 48kHz pcm audio.
>
> Please comment, Carl Eugen

As far as I understand this patch is in response to 
https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033757.html but 
in my opinion it doesn't match Robert's use-case.

Instead the "-channel_layout 0" input option should be fixed or a 
"-write_channel_mask on/off" output option should be added to the WAV 
muxer similar to the existing option in the AVI muxer.

Regards,
Tobias
Carl Eugen Hoyos Sept. 27, 2016, 7:02 a.m. UTC | #2
2016-09-27 8:49 GMT+02:00 Tobias Rapp <t.rapp@noa-archive.com>:
> On 26.09.2016 12:39, Carl Eugen Hoyos wrote:

>> Attached patch allows to write arbitrary (mono) channel_masks
>> even for 16bit 48kHz pcm audio.
>
> As far as I understand this patch is in response to
> https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033757.html
> but in my opinion it doesn't match Robert's use-case.

Where do I claim this?

Do you believe the patch is wrong or does something
unexpected?

> Instead the "-channel_layout 0" input option should be fixed

Please do send a patch!

Carl Eugen
Tobias Rapp Sept. 27, 2016, 7:38 a.m. UTC | #3
On 27.09.2016 09:02, Carl Eugen Hoyos wrote:
> 2016-09-27 8:49 GMT+02:00 Tobias Rapp <t.rapp@noa-archive.com>:
>> On 26.09.2016 12:39, Carl Eugen Hoyos wrote:
>
>>> Attached patch allows to write arbitrary (mono) channel_masks
>>> even for 16bit 48kHz pcm audio.
>>
>> As far as I understand this patch is in response to
>> https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033757.html
>> but in my opinion it doesn't match Robert's use-case.
>
> Where do I claim this?

It was how I interpreted the "just sent a patch" in 
https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033756.html .

> Do you believe the patch is wrong or does something
> unexpected?

The patch looks fine, although I haven't actually tested it.

>> Instead the "-channel_layout 0" input option should be fixed
>
> Please do send a patch!

Some time ago I tried to work on it but it turned out to be quite 
difficult especially when adding some audio filters (amerge? pan?) to 
the pipeline. So finally I refrained from doing it. Maybe I will try 
again if I find some time.

Regards,
Tobias
Carl Eugen Hoyos Sept. 27, 2016, 7:49 a.m. UTC | #4
2016-09-27 9:38 GMT+02:00 Tobias Rapp <t.rapp@noa-archive.com>:
> On 27.09.2016 09:02, Carl Eugen Hoyos wrote:
>>
>> 2016-09-27 8:49 GMT+02:00 Tobias Rapp <t.rapp@noa-archive.com>:
>>>
>>> On 26.09.2016 12:39, Carl Eugen Hoyos wrote:
>>
>>>> Attached patch allows to write arbitrary (mono) channel_masks
>>>> even for 16bit 48kHz pcm audio.
>>>
>>> As far as I understand this patch is in response to
>>> https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033757.html
>>> but in my opinion it doesn't match Robert's use-case.
>>
>> Where do I claim this?
>
> It was how I interpreted the "just sent a patch" in
> https://ffmpeg.org/pipermail/ffmpeg-user/2016-September/033756.html .

"I just sent a patch after trying to interpret your report that
was missing command line and console output."

I only understood later what Robert needs and I believe it does
fix a (not extremely unusual) use-case.

>> Do you believe the patch is wrong or does something
>> unexpected?
>
> The patch looks fine, although I haven't actually tested it.
>
>>> Instead the "-channel_layout 0" input option should be fixed
>>
>> Please do send a patch!
>
> Some time ago I tried to work on it but it turned out to be quite difficult

> especially when adding some audio filters (amerge? pan?) to the pipeline.

This issue may have been fixed, some filters do not require a channel
layout anymore. In any case, fixing the issue for the no-filter case would
be a step forward imo.

Carl Eugen
Carl Eugen Hoyos Oct. 11, 2016, 8:13 a.m. UTC | #5
2016-09-26 12:39 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>:

> Attached patch allows to write arbitrary (mono) channel_masks
> even for 16bit 48kHz pcm audio.

I'll push this if there are no objections.

Carl Eugen
Carl Eugen Hoyos Oct. 12, 2016, 10:41 a.m. UTC | #6
2016-10-11 10:13 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2016-09-26 12:39 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>:
>
>> Attached patch allows to write arbitrary (mono) channel_masks
>> even for 16bit 48kHz pcm audio.
>
> I'll push this if there are no objections.

Patch applied.

Carl Eugen
diff mbox

Patch

diff --git a/libavformat/riffenc.c b/libavformat/riffenc.c
index 36e6ac7..8ff7923 100644
--- a/libavformat/riffenc.c
+++ b/libavformat/riffenc.c
@@ -71,6 +71,8 @@  int ff_put_wav_header(AVFormatContext *s, AVIOContext *pb,
     frame_size = av_get_audio_frame_duration2(par, par->block_align);
 
     waveformatextensible = (par->channels > 2 && par->channel_layout) ||
+                           par->channels == 1 && par->channel_layout && par->channel_layout != AV_CH_LAYOUT_MONO ||
+                           par->channels == 2 && par->channel_layout && par->channel_layout != AV_CH_LAYOUT_STEREO ||
                            par->sample_rate > 48000 ||
                            par->codec_id == AV_CODEC_ID_EAC3 ||
                            av_get_bits_per_sample(par->codec_id) > 16;