GNOME Bugzilla – Bug 710324
encodebin: Implement subtitles support
Last modified: 2018-11-03 11:26:20 UTC
Currently encodebin does not support encodebin, we should impleement its support. I attach here 2 patches aiming at implementing it.
Created attachment 257452 [details] [review] encodebin: Do not systematically fail when no encoder is found In the case we can not find any encoder for a particular stream, and if the user is requesting a pad that is already compatible with the profile, we should just go without encoder, even if the caps are not raw caps.
Created attachment 257453 [details] [review] encodebin: Implement subtitles handling
*** Bug 710322 has been marked as a duplicate of this bug. ***
Created attachment 257460 [details] [review] encodebin: Implement subtitles support
Anyone looking at reviewing this?
Review of attachment 257460 [details] [review]: Looks good in general, a test would be good to have and minor nitpick: ::: gst-libs/gst/pbutils/encoding-profile.c @@ +668,3 @@ + + guint pass; + gboolean variableframerate; Why pass and variableframerate for subtitles? They are always VFR and I'm not aware of a single multipass encoder for subtitles ;)
Thibault?
Created attachment 277233 [details] [review] encodebin: Do not systematically fail when no encoder is found In the case we can not find any encoder for a particular stream, and if the user is requesting a pad that is already compatible with the profile, we should just go without encoder, even if the caps are not raw caps.
Created attachment 277234 [details] [review] encodebin: Implement subtitles support
I tested with gst-validate and have test ready for it but I have not made any unit tests as I was inspirationless about how they should look like.
Review of attachment 277233 [details] [review]: Makes sense to me
Review of attachment 277234 [details] [review]: You'll also want to update win32/common/libgstpbutils.def ::: gst-libs/gst/pbutils/encoding-profile.c @@ +915,3 @@ + * Creates a new #GstEncodingSubtitleProfile + * + * All provided allocatable arguments will be internally copied, so can be We don't usually say this, as annotations are exposed in the documentation ::: gst-libs/gst/pbutils/encoding-profile.h @@ +98,3 @@ +/** + * GstEncodingSubtitleProfile: Why call it "subtitle" profile and not "text" ? "text" seems a bit more agnostic to me :)
> +/** > + * GstEncodingSubtitleProfile: > > Why call it "subtitle" profile and not "text" ? "text" seems a bit more > agnostic to me :) Are you saying that "Text" would be better because it's semantically neutral (could be text that's not subtitles, but metadata)? I think subtitles is a better fit for most practical use cases. And subtitles might also come in a form that's not text, but bitmaps (dvd, dvb, bluray-style subs).
And it could also be Closed Caption :) Which is neither text, nor subpictures
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/92.