[PATCH v2 0/5] MR5971: winegstreamer/video_encoder: More of encoder implementation.
-- v2: winegstreamer/video_encoder: Implement GetOutputStreamInfo. winegstreamer/video_encoder: Implement GetInputStreamInfo. winegstreamer/video_encoder: Check more attributes in SetInputType. winegstreamer/video_encoder: Introduce create_input_type. mf/tests: Add more tests for h264 encoder type attributes. https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/transform.c | 49 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index deec2c7dd92..07462aa35c5 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -3886,6 +3886,14 @@ static void test_h264_encoder(void) ATTR_UINT32(test_attr_guid, 0), {0}, }; + static const struct attribute_desc test_attributes[] = + { + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width * 2, actual_height * 2), + ATTR_RATIO(MF_MT_FRAME_RATE, 10, 1), + ATTR_UINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_MixedInterlaceOrProgressive), + ATTR_UINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_Normal), + ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 2, 1), + }; const struct attribute_desc expect_input_type_desc[] = { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), @@ -3907,7 +3915,7 @@ static void test_h264_encoder(void) ATTR_UINT32(test_attr_guid, 0), {0}, }; - static const MFT_OUTPUT_STREAM_INFO expect_output_info = {.cbSize = 0x8000}; + static const MFT_OUTPUT_STREAM_INFO expect_output_info[] = {{.cbSize = 0x8000}, {.cbSize = 0x10e00}}; MFT_REGISTER_TYPE_INFO output_type = {MFMediaType_Video, MFVideoFormat_H264}; MFT_REGISTER_TYPE_INFO input_type = {MFMediaType_Video, MFVideoFormat_NV12}; IMFMediaType *media_type; @@ -3970,7 +3978,7 @@ static void test_h264_encoder(void) check_mft_set_output_type_required(transform, output_type_desc); check_mft_set_output_type(transform, output_type_desc, S_OK); check_mft_get_output_current_type(transform, expect_output_type_desc); - check_mft_get_output_stream_info(transform, S_OK, &expect_output_info); + check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[0]); /* Input types can now be enumerated. */ i = -1; @@ -3993,6 +4001,43 @@ static void test_h264_encoder(void) check_mft_get_input_current_type(transform, expect_input_type_desc); check_mft_get_input_stream_info(transform, S_OK, NULL); + hr = MFCreateMediaType(&media_type); + ok(hr == S_OK, "MFCreateMediaType returned %#lx.\n", hr); + + /* Input type attributes should match output type attributes. */ + for (i = 0; i < ARRAY_SIZE(test_attributes); ++i) + { + winetest_push_context("attr %lu", i); + + init_media_type(media_type, input_type_desc, -1); + hr = IMFMediaType_SetItem(media_type, test_attributes[i].key, &test_attributes[i].value); + ok(hr == S_OK, "SetItem returned %#lx.\n", hr); + hr = IMFTransform_SetInputType(transform, 0, media_type, MFT_SET_TYPE_TEST_ONLY); + ok(hr == MF_E_INVALIDMEDIATYPE, "SetInputType returned %#lx.\n", hr); + + winetest_pop_context(); + } + + /* Output info cbSize will change only if we change output type frame size. */ + for (i = 0; i < ARRAY_SIZE(test_attributes); ++i) + { + winetest_push_context("attr %lu", i); + + init_media_type(media_type, output_type_desc, -1); + hr = IMFMediaType_SetItem(media_type, test_attributes[i].key, &test_attributes[i].value); + ok(hr == S_OK, "SetItem returned %#lx.\n", hr); + hr = IMFTransform_SetOutputType(transform, 0, media_type, 0); + ok(hr == S_OK, "SetOutputType returned %#lx.\n", hr); + + if (IsEqualGUID(test_attributes[i].key, &MF_MT_FRAME_SIZE)) + check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[1]); + else + check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[0]); + + winetest_pop_context(); + } + + IMFMediaType_Release(media_type); ret = IMFTransform_Release(transform); ok(ret == 0, "Release returned %lu\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/video_encoder.c | 74 ++++++++++++++++-------------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index cfdd5be9d50..c880cd062ce 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -54,6 +54,45 @@ static inline struct video_encoder *impl_from_IMFTransform(IMFTransform *iface) return CONTAINING_RECORD(iface, struct video_encoder, IMFTransform_iface); } +static HRESULT create_input_type(struct video_encoder *encoder, const GUID *subtype, IMFMediaType **out) +{ + IMFVideoMediaType *input_type; + UINT64 ratio; + UINT32 value; + HRESULT hr; + + if (FAILED(hr = MFCreateVideoMediaTypeFromSubtype(subtype, &input_type))) + return hr; + + if (FAILED(hr = IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_FRAME_SIZE, &ratio)) + || FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_FRAME_SIZE, ratio))) + goto done; + + if (FAILED(hr = IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_FRAME_RATE, &ratio)) + || FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_FRAME_RATE, ratio))) + goto done; + + if (FAILED(hr = IMFMediaType_GetUINT32(encoder->output_type, &MF_MT_INTERLACE_MODE, &value)) + || FAILED(hr = IMFVideoMediaType_SetUINT32(input_type, &MF_MT_INTERLACE_MODE, value))) + goto done; + + if (FAILED(IMFMediaType_GetUINT32(encoder->output_type, &MF_MT_VIDEO_NOMINAL_RANGE, &value))) + value = MFNominalRange_Wide; + if (FAILED(hr = IMFVideoMediaType_SetUINT32(input_type, &MF_MT_VIDEO_NOMINAL_RANGE, value))) + goto done; + + if (FAILED(IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_PIXEL_ASPECT_RATIO, &ratio))) + ratio = (UINT64)1 << 32 | 1; + if (FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_PIXEL_ASPECT_RATIO, ratio))) + goto done; + + IMFMediaType_AddRef((*out = (IMFMediaType *)input_type)); + +done: + IMFVideoMediaType_Release(input_type); + return hr; +} + static HRESULT WINAPI transform_QueryInterface(IMFTransform *iface, REFIID iid, void **out) { struct video_encoder *encoder = impl_from_IMFTransform(iface); @@ -180,10 +219,6 @@ static HRESULT WINAPI transform_GetInputAvailableType(IMFTransform *iface, DWORD IMFMediaType **type) { struct video_encoder *encoder = impl_from_IMFTransform(iface); - IMFVideoMediaType *input_type; - UINT64 ratio; - UINT32 value; - HRESULT hr; TRACE("iface %p, id %#lx, index %#lx, type %p.\n", iface, id, index, type); @@ -194,36 +229,7 @@ static HRESULT WINAPI transform_GetInputAvailableType(IMFTransform *iface, DWORD if (index >= encoder->input_type_count) return MF_E_NO_MORE_TYPES; - if (!(hr = MFCreateVideoMediaTypeFromSubtype(encoder->input_types[index], &input_type))) - return hr; - - if (FAILED(hr = IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_FRAME_SIZE, &ratio)) - || FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_FRAME_SIZE, ratio))) - goto done; - - if (FAILED(hr = IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_FRAME_RATE, &ratio)) - || FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_FRAME_RATE, ratio))) - goto done; - - if (FAILED(hr = IMFMediaType_GetUINT32(encoder->output_type, &MF_MT_INTERLACE_MODE, &value)) - || FAILED(hr = IMFVideoMediaType_SetUINT32(input_type, &MF_MT_INTERLACE_MODE, value))) - goto done; - - if (FAILED(IMFMediaType_GetUINT32(encoder->output_type, &MF_MT_VIDEO_NOMINAL_RANGE, &value))) - value = MFNominalRange_Wide; - if (FAILED(hr = IMFVideoMediaType_SetUINT32(input_type, &MF_MT_VIDEO_NOMINAL_RANGE, value))) - goto done; - - if (FAILED(IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_PIXEL_ASPECT_RATIO, &ratio))) - ratio = (UINT64)1 << 32 | 1; - if (FAILED(hr = IMFVideoMediaType_SetUINT64(input_type, &MF_MT_PIXEL_ASPECT_RATIO, ratio))) - goto done; - - IMFMediaType_AddRef((*type = (IMFMediaType *)input_type)); - -done: - IMFVideoMediaType_Release(input_type); - return hr; + return create_input_type(encoder, encoder->input_types[index], type); } static HRESULT WINAPI transform_GetOutputAvailableType(IMFTransform *iface, DWORD id, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/video_encoder.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index c880cd062ce..d852a2d0a69 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -248,8 +248,10 @@ static HRESULT WINAPI transform_GetOutputAvailableType(IMFTransform *iface, DWOR static HRESULT WINAPI transform_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) { struct video_encoder *encoder = impl_from_IMFTransform(iface); + IMFMediaType *good_input_type; GUID major, subtype; - UINT64 ratio; + BOOL result; + HRESULT hr; ULONG i; TRACE("iface %p, id %#lx, type %p, flags %#lx.\n", iface, id, type, flags); @@ -281,8 +283,12 @@ static HRESULT WINAPI transform_SetInputType(IMFTransform *iface, DWORD id, IMFM if (i == encoder->input_type_count) return MF_E_INVALIDMEDIATYPE; - if (FAILED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_SIZE, &ratio)) - || FAILED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_RATE, &ratio))) + if (FAILED(hr = create_input_type(encoder, &subtype, &good_input_type))) + return hr; + hr = IMFMediaType_Compare(good_input_type, (IMFAttributes *)type, + MF_ATTRIBUTES_MATCH_OUR_ITEMS, &result); + IMFMediaType_Release(good_input_type); + if (FAILED(hr) || !result) return MF_E_INVALIDMEDIATYPE; if (flags & MFT_SET_TYPE_TEST_ONLY) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/video_encoder.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index d852a2d0a69..512a4dcfa4d 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -44,6 +44,7 @@ struct video_encoder UINT output_type_count; IMFMediaType *input_type; + MFT_INPUT_STREAM_INFO input_info; IMFMediaType *output_type; IMFAttributes *attributes; @@ -168,8 +169,12 @@ static HRESULT WINAPI transform_GetStreamIDs(IMFTransform *iface, DWORD input_si static HRESULT WINAPI transform_GetInputStreamInfo(IMFTransform *iface, DWORD id, MFT_INPUT_STREAM_INFO *info) { - FIXME("iface %p, id %#lx, info %p.\n", iface, id, info); - return E_NOTIMPL; + struct video_encoder *encoder = impl_from_IMFTransform(iface); + + TRACE("iface %p, id %#lx, info %p.\n", iface, id, info); + + *info = encoder->input_info; + return S_OK; } static HRESULT WINAPI transform_GetOutputStreamInfo(IMFTransform *iface, DWORD id, MFT_OUTPUT_STREAM_INFO *info) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/video_encoder.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index 512a4dcfa4d..40e803a9073 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -46,6 +46,7 @@ struct video_encoder IMFMediaType *input_type; MFT_INPUT_STREAM_INFO input_info; IMFMediaType *output_type; + MFT_OUTPUT_STREAM_INFO output_info; IMFAttributes *attributes; }; @@ -94,6 +95,20 @@ done: return hr; } +static HRESULT update_output_info(struct video_encoder *encoder) +{ + UINT64 frame_size; + HRESULT hr; + + if (FAILED(hr = IMFMediaType_GetUINT64(encoder->output_type, &MF_MT_FRAME_SIZE, &frame_size))) + return hr; + + /* FIXME: It's hard to calculate the encoded output size, + * use a uncompressed size here and hope it will work. */ + return MFCalculateImageSize(&MFVideoFormat_NV12, frame_size >> 32, frame_size, + (UINT32 *)&encoder->output_info.cbSize); +} + static HRESULT WINAPI transform_QueryInterface(IMFTransform *iface, REFIID iid, void **out) { struct video_encoder *encoder = impl_from_IMFTransform(iface); @@ -179,8 +194,12 @@ static HRESULT WINAPI transform_GetInputStreamInfo(IMFTransform *iface, DWORD id static HRESULT WINAPI transform_GetOutputStreamInfo(IMFTransform *iface, DWORD id, MFT_OUTPUT_STREAM_INFO *info) { - FIXME("iface %p, id %#lx, info %p.\n", iface, id, info); - return E_NOTIMPL; + struct video_encoder *encoder = impl_from_IMFTransform(iface); + + TRACE("iface %p, id %#lx, info %p.\n", iface, id, info); + + *info = encoder->output_info; + return S_OK; } static HRESULT WINAPI transform_GetAttributes(IMFTransform *iface, IMFAttributes **attributes) @@ -327,6 +346,7 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF { IMFMediaType_Release(encoder->output_type); encoder->output_type = NULL; + memset(&encoder->output_info, 0, sizeof(encoder->output_info)); } return S_OK; } @@ -361,7 +381,7 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF /* FIXME: Add MF_MT_MPEG_SEQUENCE_HEADER attribute. */ - return S_OK; + return update_output_info(encoder); } static HRESULT WINAPI transform_GetInputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5971
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146775 Your paranoid android. === debian11 (build log) === 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins === debian11b (build log) === 0120:err:winediag:h264_encoder_create GStreamer doesn't support H.264 encoding, please install appropriate plugins
On Wed Jul 3 03:00:05 2024 +0000, Ziqing Hui wrote:
This doesn't look right and we could return a hardcoded value just as well for now.
I'm not sure where this information comes from but if it really matters maybe we need a way to get it from the actual encoder? The cbSize of output info is usually used for determining the output buffer size. So I think using a size that is large enough to hold the encoded sample will make things work. So I just use a uncompressed size here, which must bigger than the encoded h264 size. The same question goes for `MF_MT_MPEG_SEQUENCE_HEADER`, is it possible to get it from GStreamer? Or does it simply not matter? I'm not pretty sure for that. I'm planning to put it aside for now, and doing researh after we are starting to implement the actuall encoding code. But the uncompressed size doesn't match and is smaller (in this case with NV12) than the value native uses.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5971#note_74975
But the uncompressed size doesn't match and is smaller (in this case with NV12) than the value native uses.
OK, you are right. I found it's smaller.
This doesn't look right and we could return a hardcoded value just as well for now.
If just use a hardcoded value here, which value should it be? Maybe the value correspond to a 1920x1080 frame? Or just use 0x8000? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5971#note_74999
On Wed Jul 3 07:22:45 2024 +0000, Ziqing Hui wrote:
But the uncompressed size doesn't match and is smaller (in this case with NV12) than the value native uses. OK, you are right. I found it's smaller. This doesn't look right and we could return a hardcoded value just as well for now. If just use a hardcoded value here, which value should it be? Maybe the value correspond to a 1920x1080 frame? Or just use 0x8000? Yeah for instance the value native returns for a 1920x1080 frame would hopefully be enough, or if the application you are working on uses something larger, that value instead. Maybe worth adding a FIXME in any case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5971#note_75000
participants (4)
-
Marvin -
Rémi Bernon -
Ziqing Hui
-
Ziqing Hui (＠zhui)