[PATCH 1/3] mfreadwrite/reader: Add recognition of ftypeMSNV mp4 header.
Needed by Greedfall intro videos. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mfreadwrite/reader.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/mfreadwrite/reader.c b/dlls/mfreadwrite/reader.c index d26778dad36..7bcb52eaaa6 100644 --- a/dlls/mfreadwrite/reader.c +++ b/dlls/mfreadwrite/reader.c @@ -2395,6 +2395,7 @@ static HRESULT bytestream_get_url_hint(IMFByteStream *stream, WCHAR const **url) static const unsigned char wavmagic[] = { 'R', 'I', 'F', 'F',0x00,0x00,0x00,0x00, 'W', 'A', 'V', 'E', 'f', 'm', 't', ' '}; static const unsigned char wavmask[] = {0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff}; static const unsigned char isommagic[] = {0x00,0x00,0x00,0x00, 'f', 't', 'y', 'p', 'i', 's', 'o', 'm',0x00,0x00,0x00,0x00}; + static const unsigned char mp4_magic[] = {0x00,0x00,0x00,0x00, 'f', 't', 'y', 'p', 'M', 'S', 'N', 'V',0x00,0x00,0x00,0x00}; static const unsigned char mp42magic[] = {0x00,0x00,0x00,0x00, 'f', 't', 'y', 'p', 'm', 'p', '4', '2',0x00,0x00,0x00,0x00}; static const unsigned char mp4mask[] = {0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x00}; static const struct stream_content_url_hint @@ -2409,6 +2410,7 @@ static HRESULT bytestream_get_url_hint(IMFByteStream *stream, WCHAR const **url) { wavmagic, L".wav", wavmask }, { isommagic, L".mp4", mp4mask }, { mp42magic, L".mp4", mp4mask }, + { mp4_magic, L".mp4", mp4mask }, }; unsigned char buffer[4 * sizeof(unsigned int)], pattern[4 * sizeof(unsigned int)]; unsigned int i, j, length = 0, caps = 0; -- 2.33.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index c3c9051a174..a3b88529c60 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -97,7 +97,7 @@ struct wg_parser_stream struct wg_parser *parser; GstPad *their_src, *post_sink, *post_src, *my_sink; - GstElement *flip; + GstElement *flip, *videobox; GstSegment segment; struct wg_format preferred_format, current_format; @@ -1150,7 +1150,7 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) if (!strcmp(name, "video/x-raw")) { - GstElement *deinterlace, *vconv, *flip, *vconv2; + GstElement *deinterlace, *vconv, *flip, *vconv2, *videobox; /* DirectShow can express interlaced video, but downstream filters can't * necessarily consume it. In particular, the video renderer can't. */ @@ -1167,6 +1167,11 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) if (!(flip = create_element("videoflip", "good"))) goto out; + /* GStreamer uses a different alignment from native for some video formats, + * we'll need to add some padding. */ + if (!(videobox = create_element("videobox", "good"))) + goto out; + /* videoflip does not support 15 and 16-bit RGB so add a second videoconvert * to do the final conversion. */ if (!(vconv2 = create_element("videoconvert", "base"))) @@ -1179,16 +1184,20 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) gst_element_sync_state_with_parent(vconv); gst_bin_add(GST_BIN(parser->container), flip); gst_element_sync_state_with_parent(flip); + gst_bin_add(GST_BIN(parser->container), videobox); + gst_element_sync_state_with_parent(videobox); gst_bin_add(GST_BIN(parser->container), vconv2); gst_element_sync_state_with_parent(vconv2); gst_element_link(deinterlace, vconv); gst_element_link(vconv, flip); - gst_element_link(flip, vconv2); + gst_element_link(flip, videobox); + gst_element_link(videobox, vconv2); stream->post_sink = gst_element_get_static_pad(deinterlace, "sink"); stream->post_src = gst_element_get_static_pad(vconv2, "src"); stream->flip = flip; + stream->videobox = videobox; } else if (!strcmp(name, "audio/x-raw")) { -- 2.33.0
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index a3b88529c60..4cadf87ccd0 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -605,6 +605,7 @@ static NTSTATUS wg_parser_stream_enable(void *args) const struct wg_parser_stream_enable_params *params = args; struct wg_parser_stream *stream = params->stream; const struct wg_format *format = params->format; + uint32_t aligned_height; stream->current_format = *format; stream->enabled = true; @@ -621,9 +622,14 @@ static NTSTATUS wg_parser_stream_enable(void *args) gst_util_set_object_arg(G_OBJECT(stream->flip), "method", "vertical-flip"); break; + case WG_VIDEO_FORMAT_NV12: + aligned_height = (stream->current_format.u.video.height + 0xf) & ~0xf; + g_object_set(G_OBJECT(stream->videobox), "bottom", + stream->current_format.u.video.height - aligned_height, NULL); + stream->current_format.u.video.height = aligned_height; + /* fallthrough */ case WG_VIDEO_FORMAT_AYUV: case WG_VIDEO_FORMAT_I420: - case WG_VIDEO_FORMAT_NV12: case WG_VIDEO_FORMAT_UYVY: case WG_VIDEO_FORMAT_YUY2: case WG_VIDEO_FORMAT_YV12: -- 2.33.0
On 10/27/21 10:04, Rémi Bernon wrote:
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
This leaves me with a lot of questions, not least of which is "can we have tests for this"? Others are: * This will change the actual height of the image, not just its layout. Is that really correct? * Is it really correct to change the height and not the width? * Is it really correct to change the reported dimensions in all circumstances? E.g. for all formats? I'm already pretty sure that it's wrong for DirectShow...
On 10/27/21 10:22 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:04, Rémi Bernon wrote:
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
This leaves me with a lot of questions, not least of which is "can we have tests for this"? Others are:
Well, I'm not sure how to add tests? Should I add a sample video file? I think I saw one already somewhere but it doesn't look we have that many and it can grow big if we add more.
* This will change the actual height of the image, not just its layout. Is that really correct?
Yes, as far as I could check it is the case.
* Is it really correct to change the height and not the width?
Well, the width isn't an issue with this specific game, the videos are 1920x1080 (and decoded as 1920x1088). If some other need it maybe it should.
* Is it really correct to change the reported dimensions in all circumstances? E.g. for all formats? I'm already pretty sure that it's wrong for DirectShow...
I guess, not sure how we can tell which one is used. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 10/27/21 15:36, Rémi Bernon wrote:
On 10/27/21 10:22 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:04, Rémi Bernon wrote:
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
This leaves me with a lot of questions, not least of which is "can we have tests for this"? Others are:
Well, I'm not sure how to add tests? Should I add a sample video file? I think I saw one already somewhere but it doesn't look we have that many and it can grow big if we add more.
Yes, we have some test video files already, including several in dlls/quartz/tests. Ideally it should be possible to reproduce with small frame sizes and short videos (just a couple of frames).
* This will change the actual height of the image, not just its layout. Is that really correct?
Yes, as far as I could check it is the case.
* Is it really correct to change the height and not the width?
Well, the width isn't an issue with this specific game, the videos are 1920x1080 (and decoded as 1920x1088). If some other need it maybe it should.
I'll admit I don't really like the idea of not even testing unaligned widths...
* Is it really correct to change the reported dimensions in all circumstances? E.g. for all formats? I'm already pretty sure that it's wrong for DirectShow...
I guess, not sure how we can tell which one is used.
I'm not sure I understand what you mean, but in theory it should be a matter of testing what output formats are enumerated for an unaligned video (say 33x33). I'm assuming that NV12 isn't the only available format; are all of the formats aligned to 16 pixels? Does this hold for different video codecs, including uncompressed RGB or YUV video? I'm not immediately sure it makes sense to test *all* these things programmatically, although it would be nice if it ends up being possible to do so easily. Playing fast and loose with what video formats we report is one thing—applications are going to find a way to blit it to the display one way or another. Changing the video *size* is another thing, and I really want to make sure we're only doing it when we're supposed to.
On 10/27/21 10:59 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 15:36, Rémi Bernon wrote:
On 10/27/21 10:22 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:04, Rémi Bernon wrote:
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
This leaves me with a lot of questions, not least of which is "can we have tests for this"? Others are:
Well, I'm not sure how to add tests? Should I add a sample video file? I think I saw one already somewhere but it doesn't look we have that many and it can grow big if we add more.
Yes, we have some test video files already, including several in dlls/quartz/tests. Ideally it should be possible to reproduce with small frame sizes and short videos (just a couple of frames).
Okay, I'll add some.
* This will change the actual height of the image, not just its layout. Is that really correct?
Yes, as far as I could check it is the case.
* Is it really correct to change the height and not the width?
Well, the width isn't an issue with this specific game, the videos are 1920x1080 (and decoded as 1920x1088). If some other need it maybe it should.
I'll admit I don't really like the idea of not even testing unaligned widths...
Like below, it's not broken as far as we know, so it doesn't seems like it needs any fixing for the moment? (I don't really mind, just saying)
* Is it really correct to change the reported dimensions in all circumstances? E.g. for all formats? I'm already pretty sure that it's wrong for DirectShow...
I guess, not sure how we can tell which one is used.
I'm not sure I understand what you mean, but in theory it should be a matter of testing what output formats are enumerated for an unaligned video (say 33x33). I'm assuming that NV12 isn't the only available format; are all of the formats aligned to 16 pixels? Does this hold for different video codecs, including uncompressed RGB or YUV video?
I'm not immediately sure it makes sense to test *all* these things programmatically, although it would be nice if it ends up being possible to do so easily.
Playing fast and loose with what video formats we report is one thing—applications are going to find a way to blit it to the display one way or another. Changing the video *size* is another thing, and I really want to make sure we're only doing it when we're supposed to.
Yeah maybe they should be aligned too, but that could be added if we find it's needed? And if things work fine with these other formats maybe not changing them is safer. I tried changing the reported formats or ordering but that game seems to explicitly request NV12 and isn't happy if it's not reported. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 10/27/21 16:05, Rémi Bernon wrote:
On 10/27/21 10:59 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 15:36, Rémi Bernon wrote:
On 10/27/21 10:22 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:04, Rémi Bernon wrote:
Native MF aligns the NV12 planes on 16 pixels, and Greedfall expects it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
This leaves me with a lot of questions, not least of which is "can we have tests for this"? Others are:
Well, I'm not sure how to add tests? Should I add a sample video file? I think I saw one already somewhere but it doesn't look we have that many and it can grow big if we add more.
Yes, we have some test video files already, including several in dlls/quartz/tests. Ideally it should be possible to reproduce with small frame sizes and short videos (just a couple of frames).
Okay, I'll add some.
* This will change the actual height of the image, not just its layout. Is that really correct?
Yes, as far as I could check it is the case.
* Is it really correct to change the height and not the width?
Well, the width isn't an issue with this specific game, the videos are 1920x1080 (and decoded as 1920x1088). If some other need it maybe it should.
I'll admit I don't really like the idea of not even testing unaligned widths...
Like below, it's not broken as far as we know, so it doesn't seems like it needs any fixing for the moment? (I don't really mind, just saying)
* Is it really correct to change the reported dimensions in all circumstances? E.g. for all formats? I'm already pretty sure that it's wrong for DirectShow...
I guess, not sure how we can tell which one is used.
I'm not sure I understand what you mean, but in theory it should be a matter of testing what output formats are enumerated for an unaligned video (say 33x33). I'm assuming that NV12 isn't the only available format; are all of the formats aligned to 16 pixels? Does this hold for different video codecs, including uncompressed RGB or YUV video?
I'm not immediately sure it makes sense to test *all* these things programmatically, although it would be nice if it ends up being possible to do so easily.
Playing fast and loose with what video formats we report is one thing—applications are going to find a way to blit it to the display one way or another. Changing the video *size* is another thing, and I really want to make sure we're only doing it when we're supposed to.
Yeah maybe they should be aligned too, but that could be added if we find it's needed? And if things work fine with these other formats maybe not changing them is safer.
Reporting multiple different formats with inconsistent sizes is something else that makes me nervous...
I tried changing the reported formats or ordering but that game seems to explicitly request NV12 and isn't happy if it's not reported.
By "explicitly request" I assume it's not doing anything like constructing its own NV12 format, right? I mean, I assume it's not explicitly asking a different size or alignment or anything either...? I was suggesting to test without the game; just create a custom file, load it into the source reader, and see what media types are offered.
participants (3)
-
Nikolay Sivov -
Rémi Bernon -
Zebediah Figura (she/her)