Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ffmpeg and libav compatibility fixes (e.g. forward ffmpeg compatibility fix for matroska embedded subtitles) #2525

Closed
wants to merge 37 commits into from

Conversation

Alphix
Copy link
Contributor

@Alphix Alphix commented Apr 1, 2013

Commit 2626cc4 in the ffmpeg repo
changed the codec id of matroska embedded S_TEXT/UTF-8 subtitles
from CODEC_ID_TEXT to AV_CODEC_ID_SUBRIP. That commit is part of
ffmpeg 1.0 and later.

Add the new codec id to provide forward compatibility for when
the embedded ffmpeg code is updated and/or when building with
external ffmpeg libraries.

Commit 2626cc4 in the ffmpeg repo
changed the codec id of matroska embedded S_TEXT/UTF-8 subtitles
from CODEC_ID_TEXT to AV_CODEC_ID_SUBRIP. That commit is part of
ffmpeg 1.0 and later.

Add the new codec id to provide forward compatibility for when
the embedded ffmpeg code is updated and/or when building with
external ffmpeg libraries.
@ghost ghost assigned elupus Apr 1, 2013
@Alphix
Copy link
Contributor Author

Alphix commented Apr 7, 2013

I'm not sure I follow.

First, the codec id (AV_CODEC_ID_SUBRIP) is defined in libavcodec/avcodec.h in recent versions of libavcodec.

Second, your #if seems to change nothing? Why test the micro version separately? The AV_VERSION_INT already includes the major/minor/micro version numbers?

@Alphix
Copy link
Contributor Author

Alphix commented Apr 7, 2013

Nevermind. I see, you were talking about libav as opposed to ffmpeg, not libavcodec.

Yes, AV_CODEC_ID_SUBRIP doesn't seem to be defined in libav. Your #if doesn't seem to fix that problem though? It'll just break ffmpeg compatibility when the major and/or minor version numbers are bumped?

@Alphix
Copy link
Contributor Author

Alphix commented Apr 7, 2013

Ok, I've educated myself. libav uses a zero based MICRO and ffmpeg uses a 100+ MICRO. I'll fix it...

FlyingRat added 2 commits April 7, 2013 16:36
This commit now contains the original patches sub directory:
  patches 			- Org dir that contains applied xbmc custom patches.
  patches/README-patches	- New README file with info about xbmc patches.
  patches/obsolete-patches	- New dir with obsolete xbmc patches.
lib/DllAvFormat.h
lib/Makefile.in
lib/ffmpeg/build_xbmc_win32.sh
project/Win32BuildSetup/BuildSetup.bat
project/Win32BuildSetup/buildmingwlibs.sh
tools/depends/native/gas-preprocessor-native/gas-preprocessor.pl
xbmc/DllPaths_win32.h
xbmc/cores/dvdplayer/DVDAudio.h
xbmc/cores/dvdplayer/DVDCodecs/DVDCodecs.h
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.cpp
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.h
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecLibMpeg2.cpp
xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpp
xbmc/cores/dvdplayer/DVDDemuxers/DVDDemux.h
xbmc/cores/dvdplayer/DVDPlayerAudio.h
xbmc/cores/dvdplayer/DVDStreamInfo.h
@MartijnKaijser
Copy link
Member

#2554

Add separate defines to identify whether the ffmpeg or libav version of
libavcodec is being used (to aid readability).
Provide additional checks for ffmpeg (as opposed to libav) and update
comments to reflect the dates of the API change.
This test (which presumably checks for LIBAVCODEC_VERSION_MAJOR >= 52)
seems both obsolete and incorrect. The major version is shifted 16 bits,
not 12. Major version 52 was introduced in commit
dd1c8f3 (from Sept 8, 2008) so this check
is anyhow obsolete.
This version of libavformat is from May 22, 2008
(commit 79d7836).

The first release to include it was ffmpeg 0.5 (3 March 2009).
This version of libavcodec is from Nov 10, 2008
(commit 3155716).

The first release to include it was ffmpeg 0.6 (15 June 2010).
This version of libavformat is from Oct 15, 2010
(commit 03700d3).

The first release to include it was ffmpeg 0.7.1 (24 June 2011).
Make the version check compatible with both ffmpeg and libav.
Also add comments describing when the API change was made for
both libraries.
Make the version check compatible with both ffmpeg and libav.
Also add comments describing when the API change was made for
both libraries.
@Alphix
Copy link
Contributor Author

Alphix commented Apr 10, 2013

@aballier Does this updated PR fix your nitpicks?

@FlyingRat
Copy link

@Alphix, your fix worked perfectly! Here are some binaries based on ffmpeg n1.2 and your work. /Regards, Lars.

osx: http://forum.xbmc.org/showthread.php?tid=156303&pid=1393277#pid1393277
win32, ios & atv: http://forum.xbmc.org/showthread.php?tid=156303&pid=1393505#pid1393505

@aballier
Copy link
Contributor

Thanks, it fixes even more than your original PR :)

The cleanup was long overdue since I believe it's been a while since xbmc has been requiring ffmpeg 0.10 in other parts of its code. It should make libav support possible now, or maybe it even works now.

For me it's all good, but it needs elupus' review, blessing and merge I guess

FlyingRat and others added 10 commits April 11, 2013 18:10
…rectory.

Since Jenkins is dependent of the "pdb" file, it is copied to the setup directory with the new name "XBMCSetup-%GIT_REV%-%target%.pdb"
…cc warning spew of 'linker input file unused because linking not done'
Fix crash on delete of active profile + other fixes for PR 2577
[fix] actually care about 12/24h clock in last introduced timeformats
move the new 'auto login' string to the xbmc language file
@elupus
Copy link
Contributor

elupus commented Apr 11, 2013

If this still compiles and work with current master (after ffmpeg bump), it can be merged.

Commit 2626cc4 in the ffmpeg repo
changed the codec id of matroska embedded S_TEXT/UTF-8 subtitles
from CODEC_ID_TEXT to AV_CODEC_ID_SUBRIP. That commit is part of
ffmpeg 1.0 and later.

Add the new codec id to provide forward compatibility for when
the embedded ffmpeg code is updated and/or when building with
external ffmpeg libraries.
Add separate defines to identify whether the ffmpeg or libav version of
libavcodec is being used (to aid readability).
Provide additional checks for ffmpeg (as opposed to libav) and update
comments to reflect the dates of the API change.
This test (which presumably checks for LIBAVCODEC_VERSION_MAJOR >= 52)
seems both obsolete and incorrect. The major version is shifted 16 bits,
not 12. Major version 52 was introduced in commit
dd1c8f3 (from Sept 8, 2008) so this check
is anyhow obsolete.
This version of libavformat is from May 22, 2008
(commit 79d7836).

The first release to include it was ffmpeg 0.5 (3 March 2009).
This version of libavcodec is from Nov 10, 2008
(commit 3155716).

The first release to include it was ffmpeg 0.6 (15 June 2010).
This version of libavformat is from Oct 15, 2010
(commit 03700d3).

The first release to include it was ffmpeg 0.7.1 (24 June 2011).
Make the version check compatible with both ffmpeg and libav.
Also add comments describing when the API change was made for
both libraries.
Make the version check compatible with both ffmpeg and libav.
Also add comments describing when the API change was made for
both libraries.
The defined(USE_EXTERNAL_FFMPEG) special case is no longer necessary
since the ffmpeg 1.2 merge.
…mpeg-fixes

Conflicts:
	xbmc/cores/dvdplayer/DVDCodecs/Overlay/DVDOverlayCodecFFmpeg.cpp
@Alphix
Copy link
Contributor Author

Alphix commented Apr 11, 2013

Gah, I messed up the PR

@Alphix
Copy link
Contributor Author

Alphix commented Apr 11, 2013

See PR #2597 instead

@Alphix Alphix closed this Apr 11, 2013
@Alphix Alphix deleted the ffmpeg-fixes branch April 20, 2013 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet