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

Hide video sink for audio files #273

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Conversation

f0k
Copy link
Contributor

@f0k f0k commented Mar 9, 2023

The canonical way to include sound files in LaTeX beamer is to use the \movie command of the multimedia package. This also works in pympress, however, it replaces the text link (or whatever is used) with a black box while the audio is playing. See the attached minimal working example for a .tex, .ogg and .pdf file displaying that behavior: test.zip

This PR is a straightforward fix:

  • In the GstOverlay, define a media_type attribute that is overridden during construction (which calls set_file). I followed the original code to create a class attribute media_type that is later shadowed by an instance attribute. (Which may or may not have been intentional, see https://www.bruceeckel.com/2022/05/11/misunderstanding-python-class-attributes/. It's also not done consistently, e.g., sink does not have a corresponding class attribute. I'd also be fine removing the class attribute and setting an instance attribute in the constructor instead.)
  • In set_file, ask Gio to guess the mime type from the file extension. This works fine as long as the extension is sane. If a user has a video file with an audio extension, it will be mistaken for an audio file, but I guess we could live with that.
  • In on_play, only unhide the video sink if the mime type does not start with audio. This is more defensive than only showing the video sink if the mime type starts with video, so it should not break things when mime type guessing did not work.

Let me know if you'd like to see any changes! And thank you for maintaining pympress, I think it solves all the issues I had with pdfpc or evince 🎉

Cimbali added a commit that referenced this pull request Mar 9, 2023
@Cimbali Cimbali merged commit 4f9d103 into Cimbali:master Mar 9, 2023
@Cimbali
Copy link
Owner

Cimbali commented Mar 9, 2023

Thanks for the PR! Real clean, didn’t know Gio had that content_type_guess.

I’ve just moved it up to the base class so all overlays can profit from it. Probably not very useful to the gif backend but now we achieve the same on the VLC one.

I followed the original code to create a class attribute media_type that is later shadowed by an instance attribute

Thanks. I’m aware of the issue but never bothered to fix it. I’ve opened #274 to discuss it.

It's also not done consistently, e.g., sink does not have a corresponding class attribute.

That’s an oversight. I’ve added the missing one.

@Cimbali
Copy link
Owner

Cimbali commented Mar 9, 2023

And thank you for maintaining pympress, I think it solves all the issues I had with pdfpc or evince

Also good to know we do some things better and it’s just not whichever software people stumble upon first.

@f0k f0k deleted the hide-audio-sink branch March 10, 2023 14:09
@f0k
Copy link
Contributor Author

f0k commented Mar 10, 2023

Wow, thanks for the quick merge!

didn’t know Gio had that content_type_guess.

Copied it from a similar hack I did for pdfpc. Only later I learned about mimetypes.guess_type, I guess that would have been the more pythonic version...

I’ve just moved it up to the base class so all overlays can profit from it.

Thank you! I can currently only test gstreamer and didn't want to be risky :)

@f0k f0k mentioned this pull request Mar 10, 2023
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