After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752186 - gst-libav: avaudenc: fails at 2 channel wmav2 file
gst-libav: avaudenc: fails at 2 channel wmav2 file
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other All
: Normal blocker
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-09 17:00 UTC by Thiago Sousa Santos
Modified: 2015-07-27 18:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
avauddec: do not fail when no channel layout was provided (1.04 KB, patch)
2015-07-09 17:01 UTC, Thiago Sousa Santos
reviewed Details | Review
avauddec: Read channels from the channels field (920 bytes, patch)
2015-07-25 06:56 UTC, Olivier Crête
none Details | Review
avauddec: Read channels from the channels field (922 bytes, patch)
2015-07-25 07:08 UTC, Olivier Crête
committed Details | Review

Description Thiago Sousa Santos 2015-07-09 17:00:37 UTC
Fails because the file has 2 channels but no channel layout was provided.

From a quick look between the differences in libav and ffmpeg it seems that libav had a default channel layout function that would provide a LEFT|RIGHT layout as default for stereo audio while ffmpeg just returns 0 (no layout defined).

I can host the file somewhere if anyone is interested in taking a deeper look at the issue.
Comment 1 Thiago Sousa Santos 2015-07-09 17:01:06 UTC
Created attachment 307163 [details] [review]
avauddec: do not fail when no channel layout was provided

ffmpeg seems to not provide channel layout for some default settings
(2 channels, for example) and avauddec should not fail on that but
try to move forward.
Comment 2 Thiago Sousa Santos 2015-07-09 17:08:55 UTC
The question is whether we want to just use channels from the avcontext and leave the layout as 0 or just do that for 1 or 2 channels where a default layout is implicit.
Comment 3 Sebastian Dröge (slomo) 2015-07-09 17:11:50 UTC
Review of attachment 307163 [details] [review]:

::: ext/libav/gstavauddec.c
@@ +381,3 @@
   if (channels == 0)
+    /* ffmpeg might not have default channel layout for some channels */
+    channels = context->channels;

The problem here is that in theory the number of channels in the context might be different than the number of channels in the current output frame. Like what we had in the video decoder before, which then caused crashes.

The context values are always for the latest input data, which might be newer values than what you currently get as output from the decoder.
Comment 4 Sebastian Dröge (slomo) 2015-07-09 17:12:09 UTC
This should IMHO be solved by fixing ffmpeg :)
Comment 5 Jan Schmidt 2015-07-23 12:59:52 UTC
Did anyone file upstream?
Comment 6 Thiago Sousa Santos 2015-07-23 16:09:31 UTC
It seems to have been caused by 464f94b206b041fa383ab4257226cb3f18dfb550, but I didn't follow up on that.
Comment 7 Olivier Crête 2015-07-25 06:56:15 UTC
Created attachment 308117 [details] [review]
avauddec: Read channels from the channels field

Turns out  it's not a ffmpeg bug, it's gst-libav not using the
API correctly, there is a channels field with the channel
couunt. Having no layout means the same thing it does in GStreamer.

If there is no layout, just read the channel count from the
channels field.
Comment 8 Olivier Crête 2015-07-25 07:08:23 UTC
Created attachment 308118 [details] [review]
avauddec: Read channels from the channels field

Slighly rebased patch
Comment 9 Sebastian Dröge (slomo) 2015-07-25 18:36:49 UTC
Comment on attachment 308118 [details] [review]
avauddec: Read channels from the channels field

This API (and the field in AVFrame) did not exist in libav when I did this changes, so we are probably removing libav compatibility with this commit now.

libav developers told me that I can assume the channel layout field to always be set.
Comment 10 Olivier Crête 2015-07-25 18:52:16 UTC
Do we care enough to want some kind of #ifdef FFMPEG or #ifdef LIBAV ?
Comment 11 Olivier Crête 2015-07-27 18:19:06 UTC
We don't care so much about libav, merged:

commit 3b6c656e2552c936a17ff6bb9a0e753fcc42e4ce
Author: Olivier Crete <olivier.crete@collabora.com>
Date:   Sat Jul 25 02:54:20 2015 -0400

    avauddec: Read channels from the channels field
    
    If there is no layout, just read the channel count from the
    channels field.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=752186