doesn't build with ffmpeg 3.0

[a-detiste]

Hi,

The debian build daemons tried to automatically rebuild Corsix-TH with FFMpeg 3.0.
That failed. Fix seems obvious ( s/av_free_packet/av_packet_unref/g),
can you please review it (I have a 6hours commute for now, sorry)
and include it in 0.60 release ?

This commit at MPV project says this api had been deprecated for a while
and /av_packet_unref is already present in FFmpeg for a while,
and that is dated Oct 2015.

mpv-player/mpv@3c081df

Replace deprecated av_free_packet() calls 

av_free_packet() got finally deprecated. Use av_packet_unref() instead,
which has almost the same semantics, has existed for a while, and is
available in all FFmpeg and Libav versions we support.

---

@emorrp1

https://buildd.debian.org/status/package.php?p=corsix-th#problem-3

xbmc/xbmc@3774381

andrewrk/libgroove@aed5104

[TheCycoONE]

Curious, I'm aware of the deprecation, but it built fine with ffmpeg 3.0 for me. There are several functions relating to packets that are deprecated as the memory model was changed so that instead of owning buffers packets share buffers and reference count them.

https://github.com/FFmpeg/FFmpeg/blob/bb9f7bf1a21d6e00dcb2afaf94d8f84e410cf89c/doc/APIchanges#L68

I would have made the change already but I haven't yet verified if our use of av_dup_packet is a noop or needs to be av_packet_ref.

[a-detiste]

but it built fine with ffmpeg 3.0 for me

Thanks for testing.

Well, build deamon did failed to rebuild 0.50, not the current git master; but all the deprecated "av_free_packet()" are still there.
Some other deprecated API have already been updated in the 0.50 -> 0.60 timeframe:
5710f1c

So there are two issues there:

• replace the deprecated API before 0.60 release to silence all these warnings
• have something that can build with FFmpeg 3.0 for Debian:
  • we could just wait on 0.60 release
  • or upload a build with videos temporary disabled if there's a hurry
  • or even write a verry short-lived patch, but that would be a waste of time.

@emorrp1

[TheCycoONE]

The commit referenced above was part of 0.50.

9260930 is probably the one you needed; 4ae56e1 is also a required patch (or else movies will segfault on newer ffmpeg); and 5e1ea3a is required for AMD users.

I think you could apply git diff v0.50..v0.60-beta2 -- CorsixTH/Src/th_movie.* as a quick patch in isolation from the rest of 0.60 if you needed something.

[emorrp1]

I've looked into the build log (search "Building CXX object") and while there are deprecation warnings (see below), that's not the actual build error.

CorsixTH/Src/th_movie.h:69:5: error: 'PixelFormat' does not name a type

Which looks like it was fixed cleanly in v0.60-beta1 (9260930) so apologies for bothering you and we should just be able to cherry-pick the patch for compatibility (or wait for the v0.60 release). I think this issue can be closed.

Use AVPixelFormat and related

Non AV prefixed PixelFormat and PIX_FMT_* enums have been deprecated for years.
They have been removed in git for both libav and ffmpeg. This patch uses the newer replacements while retaining compatibility for old ffmpeg and libav versions.

Full list of deprecations from build log (which would be nice to fix anyway):

• avpicture_get_size
• avpicture_fill
• av_dup_packet
• av_free_packet

@a-detiste FYI, I've begun preliminary changes for v0.60 in the "debian/experimental" git branch

[TheCycoONE]

If you don't mind I'd like to leave this open to cover the av_*_packet deprecation.

[emorrp1]

I see our messages got crossed, thank you for looking into it and for your dedication to quality.

9260930 is probably the one you needed

Thanks, I've backported this commit to v0.50 which will hopefully be uploaded at some point

4ae56e1 is also a required patch (or else movies will segfault on newer ffmpeg)

Indeed, we already had installed this one in the initial release

5e1ea3a is required for AMD users.

Yes, I was affected by this, but I've not backported it as the change is non-trivial (to me), the game otherwise runs fine and v0.60 will be out soon enough.

For the record, this was also reported as Debian bug #821415

[a-detiste]

I confused the depreciation warnings with actual errors. I need more sleep 😓 😄

[a-detiste]

Indeed, we already had installed this one in the initial release

I found this one in Arch Linux packaging.

[TheCycoONE]

All deprecation warnings have been resolved for 0.60. Do you guys have any other reason to keep this open (e.g. for 0.50)?

[a-detiste]

there's now 0.50-2 in Debian; 0.50-1 got accepted after the Ubuntu import freeze and thus hasn't been imported yet, so there's nothing to fix there