GNOME Bugzilla – Bug 738526
vc1parse: implement some stream-format conversion
Last modified: 2014-11-08 18:01:49 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.
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.
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.
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.
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
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.
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
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. :)
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
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
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
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.
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.
Created attachment 288961 [details] [review] vc1parse: check output format at negotiation time Updated patch to make the "git am" work.
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.
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.
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
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.
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.