After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 738526 - vc1parse: implement some stream-format conversion
vc1parse: implement some stream-format conversion
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-14 13:10 UTC by Aurélien Zanelli
Modified: 2014-11-08 18:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vc1parse: introduce a helper to make sequence-layer (8.21 KB, patch)
2014-10-14 13:16 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: prepare the stream-format conversion code and add the simplest ones (8.40 KB, patch)
2014-10-14 13:18 UTC, Aurélien Zanelli
needs-work Details | Review
vc1parse: add some simple stream-format conversion (4.28 KB, patch)
2014-10-14 13:19 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: add some asf related stream-format conversions (5.94 KB, patch)
2014-10-14 13:20 UTC, Aurélien Zanelli
needs-work Details | Review
vc1parse: check output format at negotiation time (7.31 KB, patch)
2014-10-14 13:27 UTC, Aurélien Zanelli
accepted-commit_now Details | Review
vc1parse: implement asf to *-frame-layer stream-format (7.31 KB, patch)
2014-10-14 13:28 UTC, Aurélien Zanelli
needs-work Details | Review
vc1parse: set seq_layer_sent to FALSE on reset() (845 bytes, patch)
2014-10-14 13:29 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: prepare the stream-format conversion code and add the simplest ones (8.41 KB, patch)
2014-10-20 16:00 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: add some asf related stream-format conversions (5.78 KB, patch)
2014-10-20 16:04 UTC, Aurélien Zanelli
needs-work Details | Review
vc1parse: check output format at negotiation time (7.31 KB, patch)
2014-10-20 16:05 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: implement asf to *-frame-layer stream-format (7.12 KB, patch)
2014-10-20 16:09 UTC, Aurélien Zanelli
needs-work Details | Review
vc1parse: add some asf related stream-format conversions (5.77 KB, patch)
2014-11-07 10:57 UTC, Aurélien Zanelli
committed Details | Review
vc1parse: implement asf to *-frame-layer stream-format (7.00 KB, patch)
2014-11-07 10:58 UTC, Aurélien Zanelli
committed Details | Review

Description Aurélien Zanelli 2014-10-14 13:10:01 UTC
Currently, we don't support stream-format conversion in vc1parse element so output format is the same as in input.
Most VC1 files I have seen are encapsulated into ASF container so stream-format is ASF. Unfortunately, my VC1 hardware decoder doesn't support asf stream-format in advanced profile.

So I worked on the implementation of stream-format conversion in vc1parse. I currently have a set of patches I would like to submit for review.
Comment 1 Aurélien Zanelli 2014-10-14 13:16:15 UTC
Created attachment 288510 [details] [review]
vc1parse: introduce a helper to make sequence-layer

This patch add an helper to make a sequence-layer from sequence-header. It will be useful later.
Comment 2 Aurélien Zanelli 2014-10-14 13:18:06 UTC
Created attachment 288511 [details] [review]
vc1parse: prepare the stream-format conversion code and add the simplest ones

It prepares the template for stream-format conversion and it implements the following conversion:
- sequence-layer-bdu --> bdu
- sequence-layer-bdu-frame --> bdu-frame
- sequence-layer-frame-layer --> frame-layer

The work is done in the pre_push_frame() method.
Comment 3 Aurélien Zanelli 2014-10-14 13:19:39 UTC
Created attachment 288512 [details] [review]
vc1parse: add some simple stream-format conversion

This commit add the support of following stream-format conversion:
- bdu --> sequence-layer-bdu
- bdu-frame --> sequence-layer-bdu-frame
- frame-layer --> sequence-layer-frame-layer

For these conversion, we just push a sequence-layer prior to data.
Comment 4 Aurélien Zanelli 2014-10-14 13:20:42 UTC
Created attachment 288513 [details] [review]
vc1parse: add some asf related stream-format conversions

This commit introduces an helper to convert an ASF frame to BDUs format with startcodes and use this helper to implements following stream-format conversions:
- asf --> bdu
- asf --> sequence-layer-bdu
- asf --> sequence-layer-raw-frame
Comment 5 Aurélien Zanelli 2014-10-14 13:27:48 UTC
Created attachment 288514 [details] [review]
vc1parse: check output format at negotiation time

After a discussion with Sebastian on IRC, vc1parse should fail at negotiation if we don't implement the stream-format conversion.
So this commit add an helper to check that conversion is possible and that output format is allowed at negotation.
Comment 6 Aurélien Zanelli 2014-10-14 13:28:27 UTC
Created attachment 288515 [details] [review]
vc1parse: implement asf to *-frame-layer stream-format

This commit add an helper to convert a frame to frame-layer format and use it to implement these two stream-format conversion:
- asf --> sequence-layer-frame-layer
- asf --> frame-layer
Comment 7 Aurélien Zanelli 2014-10-14 13:29:51 UTC
Created attachment 288516 [details] [review]
vc1parse: set seq_layer_sent to FALSE on reset()

I forget to reset seq_layer_sent, so this patch fix it (it may be squashed with commit which introduce it)

It is the last patch of my set. :)
Comment 8 Sebastian Dröge (slomo) 2014-10-20 11:19:22 UTC
Review of attachment 288511 [details] [review]:

::: gst/videoparsers/gstvc1parse.c
@@ +1576,3 @@
+conversion_not_supported:
+  GST_WARNING_OBJECT (vc1parse, "stream conversion not implemented yet");
+  return GST_FLOW_ERROR;

GST_FLOW_NOT_NEGOTIATED, but this should fail during negotiation already if possible. That is, the CAPS query should not claim that we can convert everything
Comment 9 Sebastian Dröge (slomo) 2014-10-20 11:22:32 UTC
Review of attachment 288513 [details] [review]:

::: gst/videoparsers/gstvc1parse.c
@@ +1379,3 @@
+  if (vc1parse->profile == GST_VC1_PROFILE_SIMPLE) {
+    GST_ERROR_OBJECT (vc1parse, "can't convert to bdu in simple profile");
+    ret = GST_FLOW_ERROR;

GST_FLOW_NOT_NEGOTIATED and it should also fail negotiation already as mentioned for the other patch
Comment 10 Sebastian Dröge (slomo) 2014-10-20 11:25:37 UTC
Review of attachment 288515 [details] [review]:

Something I also noticed in one of the other patches above

::: gst/videoparsers/gstvc1parse.c
@@ +1664,3 @@
+
+  new_buf = gst_byte_writer_reset_and_get_buffer (&bw);
+  ok &= gst_buffer_copy_into (new_buf, buffer, GST_BUFFER_COPY_METADATA, 0, -1);

Instead of copying the complete buffer you can also just prepend the extra data in front of the buffer as another GstMemory, while keeping the original GstMemory untouched
Comment 11 Aurélien Zanelli 2014-10-20 16:00:27 UTC
Created attachment 288959 [details] [review]
vc1parse: prepare the stream-format conversion code and add the simplest ones

(In reply to comment #8)

Updated patch to return GST_FLOW_NOT_NEGOTIATED instead of GST_FLOW_ERROR.

By the way, it should fail at negotiation with patch "vc1parse: check output format at negotiation time"

The query CAPS modification may be done in a separate patch.
Comment 12 Aurélien Zanelli 2014-10-20 16:04:29 UTC
Created attachment 288960 [details] [review]
vc1parse: add some asf related stream-format conversions

Change the way we convert from asf to bdu. Now instead of copying the whole buffer just to prepend 4 bytes. We just prepend the four bytes memory to the frame buffer.

Also changes the error code in case we try to do the conversion in simple profile.
Comment 13 Aurélien Zanelli 2014-10-20 16:05:28 UTC
Created attachment 288961 [details] [review]
vc1parse: check output format at negotiation time

Updated patch to make the "git am" work.
Comment 14 Aurélien Zanelli 2014-10-20 16:09:30 UTC
Created attachment 288962 [details] [review]
vc1parse: implement asf to *-frame-layer stream-format

Light rework of convert_to_frame_layer method.
- frame_layer_size is set to 8 by default and increase according to profile and keyframe instead of setting it for each cases.
- Just prepend memory to frame buffer instead of copying the whole buffer into a new one.
Comment 15 Sebastian Dröge (slomo) 2014-11-07 09:34:16 UTC
Review of attachment 288960 [details] [review]:

::: gst/videoparsers/gstvc1parse.c
@@ +1426,3 @@
+
+  startcode = GST_READ_UINT32_BE (minfo.data);
+  gst_buffer_unmap (buffer, &minfo);

You don't check here if you actually have 4 bytes to read the start code. Also you could use gst_buffer_extract(), then you don't need to map and unmap yourself.
Comment 16 Sebastian Dröge (slomo) 2014-11-07 09:36:32 UTC
Review of attachment 288962 [details] [review]:

::: gst/videoparsers/gstvc1parse.c
@@ +1680,3 @@
+
+  startcode = GST_READ_UINT32_BE (minfo.data);
+  gst_buffer_unmap (buffer, &minfo);

Same here
Comment 17 Aurélien Zanelli 2014-11-07 10:57:54 UTC
Created attachment 290149 [details] [review]
vc1parse: add some asf related stream-format conversions

Use gst_buffer_extract() to avoid map/unmap by hand and get the first four bytes.
By doing this, we also check we have four bytes.

If frame is smaller than four bytes, we just prepend start code. This can happen for instance when we have a black video sequence.
Comment 18 Aurélien Zanelli 2014-11-07 10:58:56 UTC
Created attachment 290150 [details] [review]
vc1parse: implement asf to *-frame-layer stream-format

Use gst_buffer_extract to get the first four bytes and check the returned size.