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 772067 - fdkaacenc: fix enc ! dec case
fdkaacenc: fix enc ! dec case
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.9.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-27 15:18 UTC by Vincent Penquerc'h
Modified: 2016-09-30 07:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix build error with master libfdkaac (1.02 KB, patch)
2016-09-27 15:20 UTC, Vincent Penquerc'h
none Details | Review
do not error out on out of sync error code (1.04 KB, patch)
2016-09-27 15:21 UTC, Vincent Penquerc'h
none Details | Review
set transmux on the encoder object (1.12 KB, patch)
2016-09-27 15:21 UTC, Vincent Penquerc'h
committed Details | Review
do not error out on out of sync error code (1.29 KB, patch)
2016-09-27 15:39 UTC, Vincent Penquerc'h
committed Details | Review
fix build error with master libfdkaac (1.46 KB, patch)
2016-09-27 16:15 UTC, Vincent Penquerc'h
committed Details | Review

Description Vincent Penquerc'h 2016-09-27 15:18:59 UTC
fix enc/dec case
Comment 1 Vincent Penquerc'h 2016-09-27 15:20:53 UTC
Created attachment 336369 [details] [review]
fix build error with master libfdkaac
Comment 2 Vincent Penquerc'h 2016-09-27 15:21:26 UTC
Created attachment 336370 [details] [review]
do not error out on out of sync error code
Comment 3 Vincent Penquerc'h 2016-09-27 15:21:55 UTC
Created attachment 336371 [details] [review]
set transmux on the encoder object
Comment 4 Sebastian Dröge (slomo) 2016-09-27 15:28:37 UTC
Comment on attachment 336369 [details] [review]
fix build error with master libfdkaac

MPEG2 and MPEG4 AAC are not compatible though. What is it producing now? You probably want to remove the other then...
Comment 5 Sebastian Dröge (slomo) 2016-09-27 15:29:42 UTC
Comment on attachment 336370 [details] [review]
do not error out on out of sync error code

You need to finish/drop the frame if that happens
Comment 6 Vincent Penquerc'h 2016-09-27 15:34:32 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> Comment on attachment 336369 [details] [review] [review]
> fix build error with master libfdkaac
> 
> MPEG2 and MPEG4 AAC are not compatible though. What is it producing now? You
> probably want to remove the other then...

The change in libfdkaac had duplicate tests for the MP2 and non-MP2 cases, and dropped the MP2 ones. I expect it makes the different in another way, eg:


-  if ((audioObjectType==AOT_AAC_LC)     || (audioObjectType==AOT_SBR)     || (audioObjectType==AOT_PS)    ||
-      (audioObjectType==AOT_MP2_AAC_LC) || (audioObjectType==AOT_MP2_SBR) || (audioObjectType==AOT_MP2_PS) ) {
+  if ( (audioObjectType==AOT_AAC_LC) || (audioObjectType==AOT_SBR) || (audioObjectType==AOT_PS) ) {

The commit message says:
       - Code clean up. Remove unneeded pseudo audio object types and transport types.

(AOT_MP2_AAC_LC is listed as a pseudo type in the header).
Comment 7 Vincent Penquerc'h 2016-09-27 15:39:52 UTC
Created attachment 336372 [details] [review]
do not error out on out of sync error code
Comment 8 Sebastian Dröge (slomo) 2016-09-27 15:44:53 UTC
(In reply to Vincent Penquerc'h from comment #6)
> (In reply to Sebastian Dröge (slomo) from comment #4)
> > Comment on attachment 336369 [details] [review] [review] [review]
> > fix build error with master libfdkaac
> > 
> > MPEG2 and MPEG4 AAC are not compatible though. What is it producing now? You
> > probably want to remove the other then...
> 
> The change in libfdkaac had duplicate tests for the MP2 and non-MP2 cases,
> and dropped the MP2 ones. I expect it makes the different in another way, eg:
> [...]

And how? We probably need to provide another setting to the encoder then :) Or maybe MPEG2 is just not supported at all?
Comment 9 Vincent Penquerc'h 2016-09-27 15:47:44 UTC
The doc mentions MPEG-2. But AFAICT, the patch in libfdkaac just removed all those uses of the MP2 pseudo types, with no replacement code.
Comment 10 Sebastian Dröge (slomo) 2016-09-27 16:09:57 UTC
Then we should probably remove the MPEG2 parts?
Comment 11 Vincent Penquerc'h 2016-09-27 16:15:01 UTC
It looks like MPEG2 is not supported anymore indeed:
https://github.com/nu774/fdkaac/commit/c8cc3fb57ed0eff3f16a5295d2d91dfcbf2371fd

I'll change the caps to match.
Comment 12 Vincent Penquerc'h 2016-09-27 16:15:33 UTC
Created attachment 336377 [details] [review]
fix build error with master libfdkaac
Comment 13 Vincent Penquerc'h 2016-09-27 16:21:27 UTC
commit 28a5826fa4d89f849320fe5a920afb58f7fa5714
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Tue Sep 27 16:17:00 2016 +0100

    fdkaacenc: set transmux on the fdkaac lib
    
    Not doing so will fail to decode in a simple fdkaacenc ! fdkaacdec
    pipeline, though would work if this goes through a file.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772067

commit fed624a20868fa0adbec11ba059de385ee10ad81
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Tue Sep 27 16:15:44 2016 +0100

    fdkaacdec: do not error out of out of sync return
    
    The docs say we should continue feeding in data and decoding
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772067

commit a828b12ca8c0b253ada58cc5ebc96c24dcd1212d
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Tue Sep 27 09:08:51 2016 +0100

    fdkaac: fix error with AOT_MP2_AAC_LC removed from libfdkaac API
    
    AOT_MP2_AAC_LC is a "pseudo AOT" which got removed after 0.1.4,
    and maps to AOT_AAC_LC.
    
    Remove mpegversion 2 from th caps to match.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772067