[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Andreas Cadhalpun]

Control: forwarded -1 https://git.reviewboard.kde.org/r/122569

Hi Pino,

On 09.01.2016 01:33, Pino Toscano wrote:
> In data venerdì 8 gennaio 2016 21:50:33, Andreas Cadhalpun ha scritto:
>> Could you please post links to upstream discussion about this or mark
>> the bug as forwarded?
> 
> I saw these two patches:

Thanks, marking the bug as forwarded accordingly.

> https://git.reviewboard.kde.org/r/122569/

I can confirm that this patch fixes compilation with current FFmpeg git.
But it's slightly wrong:
ifdef HAVE_FFMPEG_AVCODEC_DECODE_AUDIO4
#  if LIBAVCODEC_BUILD < AV_VERSION_INT(55,28,1)
av_free(d->frame);

Since libavcodec 54.28.0 this should be av_frame_free(>frame).
Otherwise it can leak memory.

#  else
av_frame_free(>frame);
#  endif
#endif

> https://git.reviewboard.kde.org/r/122601/

This one is trivially correct and also part of the above one.

If you want to avoid having this problem again next year (or the year after
that), you also need to replace the newly deprecated av_free_packet with
av_packet_unref, which is available since libavcodec 55.25.100 / 55.16.0:
--- k3b-2.0.3a.orig/plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
+++ k3b-2.0.3a/plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
@@ -378,7 +378,11 @@ int K3bFFMpegFile::fillOutputBuffer()
 #endif
 
 if( d->packetSize <= 0 || len < 0 )
+#if LIBAVCODEC_VERSION_MAJOR >= 56
+::av_packet_unref( >packet );
+#else
 ::av_free_packet( >packet );
+#endif
 if( len < 0 ) {
 kDebug() << "(K3bFFMpegFile) decoding failed for " << m_filename;
 return -1;

Anyway, do you know how to runtime test this code path?

> they refer to the master branch of k3b, but the code in
> plugins/decoder/ffmpeg/ is the same in both the 2.0 branch (from where
> the 2.0.x releases like the latest 2.0.3a are taken) and master.
> Would it be possible for you to check them, and maybe also comment on
> the reviewboard?

I'm no KDE developer, so I don't have a login on that review board.

>>> Let me take this opportunity to "reverse the issue": is your upstream
>>> aware that the continuous API breaks in each new stable series only
>>> waste time on the users' side?
>>
>> I sense frustration in these words, but let's get the facts straight:
>>  * The last four stable series (2.5.*-2.8.*) didn't break the API.
> 
> Possibly, but IIRC only 2.7 was used back in Debian, so what 2.5 and 2.6
> did is generally not relevant for me. Before it was libav, yes, but from
> a Debian maintainer POV it was "whatever provided libav* libraries".

I just wanted to be exact here. And it's still mainly Libav were the
deprecations (and the nice new APIs) come from, which get merged into FFmpeg.
The difference here is that FFmpeg has more stable releases. ;)
You could have said "API breaks once a year" and would have been mostly
correct. [1]

>>  * Adapting to the new APIs is no waste of time, rather maintenance cost,
>>also improving the API using program in the process, as the old APIs
>>were deprecated for a reason.
> 
> The ffmpeg decoding plugin in k3b is doing the same job for the latest
> 6+ years, so at least to me all these API changes look like they brought
> nothing.

Maybe, but e.g. an mpv developer will see this quite differently.

> And well, changing the code to newer APIs that don't bring
> anything new to what was already done is a cost that, as developer, I'd
> rather not pay.

Well, tell that e.g. the Qt/Gtk developers, or try your luck with the LLVM
developers, who like to gratuitously move headers around every half
a year...
At least FFmpeg has a reasonable deprecation period (usually > 2 years).
Keeping all old API forever is just not feasible for many important
libraries.

And while we're at that, someone should probably fix all those deprecation
warnings coming from Qt...

> There is not an active k3b upstream,

That's the real problem, I'm afraid.
Seriously, the patches on the review board are almost a year old...

But why is k3b not developed anymore?
Are there better alternatives?

> so I'm afraid the new ffmpeg
> support code should be as much as conservative as possible (that is,
> support the new version but keep supporting older ones as done now).

I don't see what that has to do with having no active upstream.

What are the oldest libavcodec/libavformat versions used for compiling this?

Or are you seriously claiming anyone still uses this code with a libavformat
version from before 2005?
(I'm referring to LIBAVFORMAT_BUILD < 4629.)

Best regards,
Andreas


1: http://abi-laboratory.pro/tracker/timeline/ffmpeg/

___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras

[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Andreas Cadhalpun]

Hi Pino,

Thanks for your quick reply.

On 08.01.2016 18:12, Pino Toscano wrote:
> Yes, I've seen the patch in #807853, and a couple of review requests on
> KDE's reviewboard.

Could you please post links to upstream discussion about this or mark
the bug as forwarded?

> Let me take this opportunity to "reverse the issue": is your upstream
> aware that the continuous API breaks in each new stable series only
> waste time on the users' side?

I sense frustration in these words, but let's get the facts straight:
 * The last four stable series (2.5.*-2.8.*) didn't break the API.
 * Adapting to the new APIs is no waste of time, rather maintenance cost,
   also improving the API using program in the process, as the old APIs
   were deprecated for a reason.

And yes, the upstream developers are aware of that maintenance cost,
I made sure of that. Please go and read the discussion upstream [1][2].
And I can understand your frustration, after all, I'm the one who wrote
patches for all affected Debian packages.

> Just look at the affected code in k3b,
> plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp: many defines and ifdef's,
> just to make the very same code do the very same job, nothing less and
> nothing more.

I'm well aware how this file looks, as I wrote a patch for it.
And most of these ifdef's could just be removed, unless you need to
build this in ancient environments...

> Sure, if I don't get around to test your patch or to integrate eventual
> upstream fixes the plugin will need to be disabled, but honestly I don't
> see how this would be in favour of our users.

That's why I provided a patch, which should be more than enough to fix
this issue properly for any reasonably maintained software.

Best regards,
Andreas


1: https://lists.libav.org/pipermail/libav-devel/2015-July/071229.html
2: https://ffmpeg.org/pipermail/ffmpeg-devel/2015-July/176439.html

___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras

[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Pino Toscano]

In data venerdì 8 gennaio 2016 21:50:33, Andreas Cadhalpun ha scritto:
> Hi Pino,
> 
> Thanks for your quick reply.
> 
> On 08.01.2016 18:12, Pino Toscano wrote:
> > Yes, I've seen the patch in #807853, and a couple of review requests on
> > KDE's reviewboard.
> 
> Could you please post links to upstream discussion about this or mark
> the bug as forwarded?

I saw these two patches:
https://git.reviewboard.kde.org/r/122569/
https://git.reviewboard.kde.org/r/122601/
they refer to the master branch of k3b, but the code in
plugins/decoder/ffmpeg/ is the same in both the 2.0 branch (from where
the 2.0.x releases like the latest 2.0.3a are taken) and master.
Would it be possible for you to check them, and maybe also comment on
the reviewboard?

> > Let me take this opportunity to "reverse the issue": is your upstream
> > aware that the continuous API breaks in each new stable series only
> > waste time on the users' side?
> 
> I sense frustration in these words, but let's get the facts straight:
>  * The last four stable series (2.5.*-2.8.*) didn't break the API.

Possibly, but IIRC only 2.7 was used back in Debian, so what 2.5 and 2.6
did is generally not relevant for me. Before it was libav, yes, but from
a Debian maintainer POV it was "whatever provided libav* libraries".

>  * Adapting to the new APIs is no waste of time, rather maintenance cost,
>also improving the API using program in the process, as the old APIs
>were deprecated for a reason.

The ffmpeg decoding plugin in k3b is doing the same job for the latest
6+ years, so at least to me all these API changes look like they brought
nothing. And well, changing the code to newer APIs that don't bring
anything new to what was already done is a cost that, as developer, I'd
rather not pay.

> And yes, the upstream developers are aware of that maintenance cost,
> I made sure of that. Please go and read the discussion upstream [1][2].
> And I can understand your frustration, after all, I'm the one who wrote
> patches for all affected Debian packages.

Thank you for that, at least should help bringing upstream back to the
real world, where there are developers using older distros (e.g. LTS
ones)...

> > Just look at the affected code in k3b,
> > plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp: many defines and ifdef's,
> > just to make the very same code do the very same job, nothing less and
> > nothing more.
> 
> I'm well aware how this file looks, as I wrote a patch for it.
> And most of these ifdef's could just be removed, unless you need to
> build this in ancient environments...

There is not an active k3b upstream, so I'm afraid the new ffmpeg
support code should be as much as conservative as possible (that is,
support the new version but keep supporting older ones as done now).

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras

[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Pino Toscano]

Hi,

In data venerdì 8 gennaio 2016 01:19:01, Andreas Cadhalpun ha scritto:
> Dear Maintainer,
> 
> the next version of FFmpeg is planned to be released this month
> (and it might be called 3.0 instead of 2.9).
> 
> Since I haven't heard back from you during the past month
> I'm wondering what the status of this bug is:
>  * Are you aware of the patch I provided?
>  * Do you plan an upload soon?
>  * Is upstream aware of the problem?

Yes, I've seen the patch in #807853, and a couple of review requests on
KDE's reviewboard.

Let me take this opportunity to "reverse the issue": is your upstream
aware that the continuous API breaks in each new stable series only
waste time on the users' side? Just look at the affected code in k3b,
plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp: many defines and ifdef's,
just to make the very same code do the very same job, nothing less and
nothing more.

Sure, if I don't get around to test your patch or to integrate eventual
upstream fixes the plugin will need to be disabled, but honestly I don't
see how this would be in favour of our users.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras

[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Andreas Cadhalpun]

Dear Maintainer,

the next version of FFmpeg is planned to be released this month
(and it might be called 3.0 instead of 2.9).

Since I haven't heard back from you during the past month
I'm wondering what the status of this bug is:
 * Are you aware of the patch I provided?
 * Do you plan an upload soon?
 * Is upstream aware of the problem?

By the way, this issue had already been reported in 2014 (#739312),
when this functionality got removed from Libav.

If this bug isn't fixed soon, it will become release critical and
thus the package will either get NMUed or removed from testing.

Also, Pino, as you are regularly uploading this package, please
consider adding yourself as uploader.

Best regards,
Andreas

___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras

[Pkg-kde-extras] Bug#807853: k3b: FTBFS with FFmpeg 2.9

[Andreas Cadhalpun]

Package: k3b
Version: 2.0.3a-1
Severity: important
Tags: patch
User: pkg-multimedia-maintain...@lists.alioth.debian.org
Usertags: ffmpeg2.9

Dear Maintainer,

thanks for re-enabling the ffmpeg plugin, but unfortunately
your package fails to build with the upcoming ffmpeg 2.9.
This bug will become release-critical at some point when the
ffmpeg2.9 transition gets closer.

Attached is a patch replacing the deprecated functionality.
It also works with ffmpeg 2.8.
Please apply this patch and forward it upstream, if necessary.

These changes are non-trivial and should be runtime-tested.

Best regards,
Andreas
diff --git a/debian/patches/ffmpeg-2.9.patch b/debian/patches/ffmpeg-2.9.patch
new file mode 100644
index 000..07ee5bc
--- /dev/null
+++ b/debian/patches/ffmpeg-2.9.patch
@@ -0,0 +1,56 @@
+Description: Replace deprecated FFmpeg API
+Author: Andreas Cadhalpun 
+Last-Update: <2015-12-13>
+
+--- k3b-2.0.3a.orig/plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
 k3b-2.0.3a/plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
+@@ -329,6 +329,11 @@ int K3bFFMpegFile::fillOutputBuffer()
+ d->outputBufferPos = d->alignedOutputBuffer;
+ d->outputBufferSize = AVCODEC_MAX_AUDIO_FRAME_SIZE;
+ 
++#ifdef HAVE_FFMPEG_AVCODEC_DECODE_AUDIO4
++AVFrame *frame = av_frame_alloc();
++int got_frame = 0;
++int len = ::avcodec_decode_audio4(FFMPEG_CODEC(d->formatContext->streams[0]), frame, _frame, >packet);
++#else
+ #ifdef HAVE_FFMPEG_AVCODEC_DECODE_AUDIO3
+ int len = ::avcodec_decode_audio3(
+ #else
+@@ -347,14 +352,24 @@ int K3bFFMpegFile::fillOutputBuffer()
+ #else
+ d->packetData, d->packetSize );
+ #endif
++#endif
+ 
+ if( d->packetSize <= 0 || len < 0 )
+ ::av_free_packet( >packet );
+ if( len < 0 ) {
++#ifdef HAVE_FFMPEG_AVCODEC_DECODE_AUDIO4
++av_frame_free();
++#endif
+ kDebug() << "(K3bFFMpegFile) decoding failed for " << m_filename;
+ return -1;
+ }
+ 
++#ifdef HAVE_FFMPEG_AVCODEC_DECODE_AUDIO4
++d->outputBufferSize = av_get_bytes_per_sample(FFMPEG_CODEC(d->formatContext->streams[0])->sample_fmt) * FFMPEG_CODEC(d->formatContext->streams[0])->channels * frame->nb_samples;
++memcpy(d->alignedOutputBuffer, frame->data[0], d->outputBufferSize);
++av_frame_free();
++#endif
++
+ d->packetSize -= len;
+ d->packetData += len;
+ }
+@@ -420,9 +435,9 @@ K3bFFMpegFile* K3bFFMpegWrapper::open( c
+ // mp3 being one of them sadly. Most importantly: allow the libsndfile decoder to do
+ // its thing.
+ //
+-if( file->type() == CODEC_ID_WMAV1 ||
+-file->type() == CODEC_ID_WMAV2 ||
+-file->type() == CODEC_ID_AAC )
++if( file->type() == AV_CODEC_ID_WMAV1 ||
++file->type() == AV_CODEC_ID_WMAV2 ||
++file->type() == AV_CODEC_ID_AAC )
+ #endif
+ return file;
+ }
diff --git a/debian/patches/series b/debian/patches/series
index 9db785e..f150af0 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,4 @@
 112_dont_require_mp3.diff
 113_initial_preference.diff
 cmake-duplicate-doc.diff
+ffmpeg-2.9.patch
___
pkg-kde-extras mailing list
pkg-kde-extras@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-kde-extras