[PATCH 0/1] MR4600: winegstreamer: Set streamheader field for h264 caps.
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600
From: Ziqing Hui <zhui(a)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); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4600
@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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600#note_55016
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600#note_55097
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600#note_55159
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600#note_55162
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600#note_55318
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4600
participants (4)
-
Rémi Bernon -
Zebediah Figura (＠zfigura)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)