[k3b] Review Request 113295: Fix compiling k3b against latest ffmpeg.

[Michael Pyne]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113295/#review42772
-----------------------------------------------------------



CMakeLists.txt
<http://git.reviewboard.kde.org/r/113295/#comment30935>

    libav/ffmpeg seems to have a AV_VERSION_INT() macro to do this calculation. Though I guess the question is whether that macro is present in all supported versions of libav/ffmpeg?



CMakeLists.txt
<http://git.reviewboard.kde.org/r/113295/#comment30936>

    I'm assuming the return 0; at the end of this comment is meant to be ignored to allow the shell main() function to just return immediately? If so I'd recommend removing the 'return 0;' for clarity.



plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
<http://git.reviewboard.kde.org/r/113295/#comment30938>

    I would suggest adding the #ifdef here to 
    "#define AV_CODEC_ID_FOO CODEC_ID_FOO" for old ffmpeg.
    
    Then each case statement would unconditionally be case AV_CODEC_ID_FOO instead of duplicating the switch contents.
    
    However I'm not too bugged about it either way, and the existing patch would work well.



plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp
<http://git.reviewboard.kde.org/r/113295/#comment30937>

    Trailing whitespace here.


The patch works for me (though I also have a recent ffmpeg). I have some comments, but I'd say if no one with an old ffmpeg chimes in to confirm that this still compiles there I'd go ahead and commit. The compatibility wrappers with the old code appear correct at least.

- Michael Pyne


On Oct. 16, 2013, 9:41 p.m., Michael Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113295/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2013, 9:41 p.m.)
> 
> 
> Review request for K3b.
> 
> 
> Repository: k3b
> 
> 
> Description
> -------
> 
> Fix compiling k3b against latest ffmpeg. They once again broke source compatibility.
> 
> Thats two patches. Reviewboard made it into one. The MAX_AUDIO_FRAME thing is a separate patch.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt ca1531d8e601e423002f6ffa490f2d5bb8dbf5d8 
>   plugins/decoder/ffmpeg/k3bffmpegwrapper.h 1ec36b6e465a08f7fe42c80b926e98683be75a5e 
>   plugins/decoder/ffmpeg/k3bffmpegwrapper.cpp a5ef087a5ac3a2dc8b7d385b9c3a2e16bf6622bb 
> 
> Diff: http://git.reviewboard.kde.org/r/113295/diff/
> 
> 
> Testing
> -------
> 
> Compiled against latest ffmpeg. Now i need someone with a ffpmpeg that still has CodecID to check if it still works.
> 
> 
> Thanks,
> 
> Michael Jansen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/k3b/attachments/20131101/128805d6/attachment.html>