| Message ID | 20200126151250.38764-1-federico.simoncelli@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avdevice/pulse_audio_dec: identify channel layout | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Am So., 26. Jan. 2020 um 16:23 Uhr schrieb Federico Simoncelli <federico.simoncelli@gmail.com>: > > This patch adds the attempt to identify the pulseaudio channel map and > when possible set the relevant channel_layout parameter of the codec. > > The result is an improvement over the current behavior of guessing the > layout based on the number of channels (for more information see > guess_layout_max). > --- > libavdevice/pulse_audio_dec.c | 37 +++++++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/libavdevice/pulse_audio_dec.c b/libavdevice/pulse_audio_dec.c > index 50a3c971ae..2f76cfd474 100644 > --- a/libavdevice/pulse_audio_dec.c > +++ b/libavdevice/pulse_audio_dec.c > @@ -135,6 +135,34 @@ static av_cold int pulse_close(AVFormatContext *s) > return 0; > } > > +static int pulse_map_to_layout(pa_channel_map *cmap) > +{ > + const char *map_name = pa_channel_map_to_name(cmap); > + > + if (strcmp(map_name, "mono") == 0) > + return AV_CH_LAYOUT_MONO; > + > + if (strcmp(map_name, "stereo") == 0) > + return AV_CH_LAYOUT_STEREO; > + if (strcmp(map_name, "surround-40") == 0) > + return AV_CH_LAYOUT_4POINT0; > + > + if (strcmp(map_name, "surround-41") == 0) > + return AV_CH_LAYOUT_4POINT1; > + > + if (strcmp(map_name, "surround-50") == 0) > + return AV_CH_LAYOUT_5POINT0; > + > + if (strcmp(map_name, "surround-51") == 0) > + return AV_CH_LAYOUT_5POINT1; > + > + if (strcmp(map_name, "surround-71") == 0) > + return AV_CH_LAYOUT_7POINT1; Can you confirm that with the existing code, user could set the channel_layout for everything >= 3 channels? Does this still work after your patch? Carl Eugen
On Sun, Jan 26, 2020 at 4:33 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Can you confirm that with the existing code, user could set the > channel_layout > for everything >= 3 channels? Does this still work after your patch? > > Carl Eugen Hi Carl, thanks for asking, actually I caught a segfault with >= 3 channels. I revised the patch, for the amount of testing I did (ffmpeg -f pulse -ac 3 -i default -f null -), the code seems OK now. Thank you,
> Am 26.01.2020 um 17:42 schrieb Federico Simoncelli <federico.simoncelli@gmail.com>: > > On Sun, Jan 26, 2020 at 4:33 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: >> Can you confirm that with the existing code, user could set the >> channel_layout >> for everything >= 3 channels? Does this still work after your patch? > > Hi Carl, thanks for asking, actually I caught a segfault with >= 3 > channels. Please elaborate. Carl Eugen
On Sun, Jan 26, 2020 at 6:18 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > Am 26.01.2020 um 17:42 schrieb Federico Simoncelli <federico.simoncelli@gmail.com>: > > > > On Sun, Jan 26, 2020 at 4:33 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> > > wrote: > >> Can you confirm that with the existing code, user could set the > >> channel_layout > >> for everything >= 3 channels? Does this still work after your patch? > > > > Hi Carl, thanks for asking, actually I caught a segfault with >= 3 > > channels. > > Please elaborate. The issue is fixed in the patch announced by this message and the change introduced is the check for map_name == NULL. That ensures that when we specify or force a particular number of channels (e.g. -ac 3 or -channels 3, or higher) or whenever pulse cannot provide a channel map (NULL) then we fallback to the regular channel guessing in place before (channel_layout = 0).
diff --git a/libavdevice/pulse_audio_dec.c b/libavdevice/pulse_audio_dec.c index 50a3c971ae..2f76cfd474 100644 --- a/libavdevice/pulse_audio_dec.c +++ b/libavdevice/pulse_audio_dec.c @@ -135,6 +135,34 @@ static av_cold int pulse_close(AVFormatContext *s) return 0; } +static int pulse_map_to_layout(pa_channel_map *cmap) +{ + const char *map_name = pa_channel_map_to_name(cmap); + + if (strcmp(map_name, "mono") == 0) + return AV_CH_LAYOUT_MONO; + + if (strcmp(map_name, "stereo") == 0) + return AV_CH_LAYOUT_STEREO; + + if (strcmp(map_name, "surround-40") == 0) + return AV_CH_LAYOUT_4POINT0; + + if (strcmp(map_name, "surround-41") == 0) + return AV_CH_LAYOUT_4POINT1; + + if (strcmp(map_name, "surround-50") == 0) + return AV_CH_LAYOUT_5POINT0; + + if (strcmp(map_name, "surround-51") == 0) + return AV_CH_LAYOUT_5POINT1; + + if (strcmp(map_name, "surround-71") == 0) + return AV_CH_LAYOUT_7POINT1; + + return 0; +} + static av_cold int pulse_read_header(AVFormatContext *s) { PulseData *pd = s->priv_data; @@ -245,10 +273,11 @@ static av_cold int pulse_read_header(AVFormatContext *s) pa_threaded_mainloop_unlock(pd->mainloop); /* take real parameters */ - st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; - st->codecpar->codec_id = codec_id; - st->codecpar->sample_rate = pd->sample_rate; - st->codecpar->channels = pd->channels; + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; + st->codecpar->codec_id = codec_id; + st->codecpar->sample_rate = pd->sample_rate; + st->codecpar->channels = pd->channels; + st->codecpar->channel_layout = pulse_map_to_layout(&cmap); avpriv_set_pts_info(st, 64, 1, 1000000); /* 64 bits pts in us */ pd->timefilter = ff_timefilter_new(1000000.0 / pd->sample_rate,