jpegenc: remove meta copy in jpegenc

GstVideoEncoder takes care of the Meta copy, so there is no need in jpegenc

Fixes http://gstreamer-devel.966125.n4.nabble.com/jpegenc-copy-GstMeta-twice-tt4693981.html

Looks good to me, thanks !

assigned to @gstreamer-merge-bot

assigned to @yan3nian and unassigned @gstreamer-merge-bot

I couldn't merge this branch: Failed to push rebased changes, check my logs!

Ah oops, @yan3nian I have one thing needed for you, can you edit the MR and check the box that allow modification of your branch please ? It's needed by the CI both to rebase and also to edit the commit messages.

changed the description

added 5 commits

• c1428403...6a389615 - 4 commits from branch gstreamer:master
• 1e862fbf - jpegenc: remove meta copy in jpegenc

Compare with previous version

added 3 commits

• 1e862fbf...518d192d - 2 commits from branch gstreamer:master
• 1d2a9106 - jpegenc: remove meta copy in jpegenc

Compare with previous version

Hi @ndufresne sorry for the late response, I have checked the box

assigned to @gstreamer-merge-bot and unassigned @yan3nian

mentioned in commit yan3nian/gst-plugins-good@1944564d

added 4 commits

• 1d2a9106...e5feaa76 - 3 commits from branch gstreamer:master
• 1944564d - jpegenc: remove meta copy in jpegenc

Compare with previous version

merged

changed milestone to %1.17.1

This patch breaks my application that relied on jpegenc copying metadata of type GstVideoRegionOfInterestMeta. It looks like GstVideoEncoder copies only specific kinds of metadata. Do applications now need to make any changes to restore original behavior?

I must admit, I don't understand the encoder code that decide to copy or not:

https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/blob/master/gst-libs/gst/video/gstvideoencoder.c#L2145

Great, so @slomo commit explains it as "Same behaviour as in GstVideoFilter.", still make no sense to me atm.

gst-plugins-base@ceef2cc1

So this Meta has GST_META_TAG_VIDEO_SIZE_STR and GST_META_TAG_VIDEO_ORIENTATION_STR, on top of GST_META_TAG_VIDEO_STR. Reading the minimalistic doc, it says if orientation or size change, this meta is not longer relevant (I guess that means it should be discarded ?) But ironically, the meta implements a transform function for size changes (not or orientation change).

The being said, is an encoded video a video ? Should encoder really carry over meta with video tags ? This tagging design seems pretty gray to me.

It copies all metas without tags (they don't depend on any specific aspect of the buffer so are plain metadata) and metas that only have the "video" tag (they only depend on the stream being video). ROI meta also depends on the resolution, so is not copied.

Some encoders/decoders change the resolution so just copying here might not be correct. It needs special handling in the subclass.

This tagging design seems pretty gray to me

Yes, it's not well designed but all we have right now. And of course encoded video is still video, it's in the name ;)

But ironically, the meta implements a transform function for size changes (not or orientation change).

But the base-class doesn't know if any size changes are happening, so can't call the transform function like that.