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

Update to the latest FFMpeg (5.1) on Debian #72

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

sumo
Copy link

@sumo sumo commented Apr 3, 2024

Changes to support the 5.x and greater libav* versions. New encoding and decoding process was needed. I have cleaned up the PR as much as possible to align with the original coding style.

I am not a stack user at all so I don't know if the stack requirements are met - it builds with stack but I am not sure about LTS version management.

Had to remove 8.6.5 and 9.0.2 from the supported builds as I had trouble building hsc2hs and JuicyPixels using these ghc versions.

Copy link
Owner

@acowley acowley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This is an incredibly helpful contribution.

where
aux md r = runMaybeT $ do
frame <- MaybeT r
MaybeT $ toJuicyImage mdisp (displayRotation md) frame
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand the extra Bool argument we pick up in the image related functions, and I think maybe the naming is a bit confusing along this path. Above, we have toJuicyImage rotateIfPres mdisp, but here we have this mdisp parameter that is actually not the same as the mdisp parameter of toJuicyImage. I'm also a bit unclear on why we need a Bool in addition to the Maybe. Could the Nothing variant imply a False for the boolean?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Bool to imageWriterT allows the caller to not want the image rotated even if rotation metadata is present. I can clean up the aux function for False to be Nothing so that toJuicyImage always takes a Maybe and doesn't care about the flag.

(avcodec_send_frame ctx frame)
read_pkts
return writeAudioFrame
else
return $ \_ -> return ()

videoWriter <- case mVideoStream of
vWriter <- case mVideoStream of
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for the name changes here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name clashing with the videoWriter function so the change to reduce warnings

@@ -498,7 +514,7 @@ videoWriter ep fname = do

data StreamParams =
JustVideo VideoParams
| JustAudio AudioParams
| JustAudio AudioParams
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to have picked up some spurious trailing whitespace here.

write_header_check oc

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More spurious whitespace.

@sumo
Copy link
Author

sumo commented Apr 13, 2024

I'm not too familiar with the Github CI process, should I update the ci.yml file or is that something you will do?

sumo added 2 commits April 14, 2024 18:00
…s a #define. Add alpine container build to ensure build succeeds on musl.
emilazy added a commit to emilazy/nixpkgs that referenced this pull request Aug 20, 2024
This package has been marked as broken for a year. If anyone wants to
fix it, there is a pull request updating it to the current FFmpeg API:
<acowley/ffmpeg-light#72>.
emilazy added a commit to emilazy/nixpkgs that referenced this pull request Aug 23, 2024
This package has been marked as broken for a year. If anyone wants to
fix it, there is a pull request updating it to the current FFmpeg API:
<acowley/ffmpeg-light#72>.

(cherry picked from commit 7ce990f)
RCoeurjoly pushed a commit to RCoeurjoly/nixpkgs that referenced this pull request Aug 23, 2024
This package has been marked as broken for a year. If anyone wants to
fix it, there is a pull request updating it to the current FFmpeg API:
<acowley/ffmpeg-light#72>.

(cherry picked from commit 7ce990f)
greg-hellings pushed a commit to greg-hellings/nixpkgs that referenced this pull request Aug 24, 2024
This package has been marked as broken for a year. If anyone wants to
fix it, there is a pull request updating it to the current FFmpeg API:
<acowley/ffmpeg-light#72>.

(cherry picked from commit 7ce990f)
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.

2 participants