For h264 byte-stream format, we should set "streamheader". "codec_data" is for avc.
We also set timestamp of the buffer to 0 because mp4mux will be error if the PTS is none.
From: Ziqing Hui zhui@codeweavers.com
For h264 byte-stream format, we should set "streamheader". "codec_data" is for avc.
We also set timestamp of the buffer to 0 because mp4mux will be error if the PTS is none. --- dlls/winegstreamer/wg_format.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index 274a6dec261..d0a4b6fc528 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -721,8 +721,10 @@ static GstCaps *wg_format_to_caps_video_h264(const struct wg_format *format) return NULL; }
+ GST_BUFFER_PTS(buffer) = 0; + GST_BUFFER_DTS(buffer) = 0; gst_buffer_fill(buffer, 0, format->u.video_h264.codec_data, format->u.video_h264.codec_data_len); - gst_caps_set_simple(caps, "codec_data", GST_TYPE_BUFFER, buffer, NULL); + gst_caps_set_simple(caps, "streamheader", GST_TYPE_BUFFER, buffer, NULL); gst_buffer_unref(buffer); }
@zfigura @rbernon plz review it asap. I'd like to get this upstreamed before code freeze. This patch is important for making mp4 muxer be able to work.
This merge request was approved by Rémi Bernon.
Do we actually need to set streamheader? Are there any elements that need it, or can we just revert 320383c594c4eef821ea380b52021d32c8073056?
As a side note, it would be rather nice to have more tests for the mp4 muxer, ideally tests that use realistic formats like H.264 and AAC, and tests that actually feed samples in and get data out. Testing that the data is correct is likely to be a lot harder, but currently we aren't doing any streaming at all.
Do we actually need to set streamheader? Are there any elements that need it, or can we just revert 320383c594c4eef821ea380b52021d32c8073056?
Yeah, we actually need that. byte-stream H264 need streamheader to be set or the streamheader should be sent to the element as the first data buffer. I think it's easier to set it as a streamheader field in caps because this is what PE side does: setting the h264 header in a standalone field.
As a side note, it would be rather nice to have more tests for the mp4 muxer, ideally tests that use realistic formats like H.264 and AAC, and tests that actually feed samples in and get data out. Testing that the data is correct is likely to be a lot harder, but currently we aren't doing any streaming at all.
I have a local test program[1] to produce a mp4 file with H264 video stream (audio stream is not tested though). After this patch, a playable mp4 file can be successfully consumed.
On Wed Dec 6 01:51:41 2023 +0000, Ziqing Hui wrote:
Do we actually need to set streamheader? Are there any elements that
need it, or can we just revert 320383c594c4eef821ea380b52021d32c8073056? Yeah, we actually need that. It is needed by h264parse[1], byte-stream H264 need streamheader to be set or the streamheader should be sent to the h264parse as the first data buffer. I think it's easier to set it as a streamheader field in caps because this is what PE side does: setting the h264 header in a standalone field.
As a side note, it would be rather nice to have more tests for the mp4
muxer, ideally tests that use realistic formats like H.264 and AAC, and tests that actually feed samples in and get data out. Testing that the data is correct is likely to be a lot harder, but currently we aren't doing any streaming at all. I have a local test program[1] to produce a mp4 file with H264 video stream (audio stream is not tested though). After this patch, a playable mp4 file can be successfully consumed. It fails before this patch. [1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/blob/master/gst/v... [2] [mp4.c](/uploads/d2221e2e9575973502ddc2aadcaa40b1/mp4.c) This program is also tested on Windows, it can produce a playable mp4 file on Windows too.
Also, I did submitted some tests before: !3369. I'll try to get it upstreamed.
On Wed Dec 6 02:42:30 2023 +0000, Ziqing Hui wrote:
Also, I did submitted some tests before: !3369. I'll try to get it upstreamed.
Ahh, so the tests make it clearer—Microsoft makes the unusual choice of using Annex B but providing the SPS/PPS NALUs out of band. So yes, it does look like using "streamheader" is correct here.
This merge request was approved by Zebediah Figura.
-
Rémi Bernon (＠rbernon) -
Zebediah Figura (＠zfigura)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)