[PATCH 0/6] MR5159: Overwatch highlight/POTG encoding (part 1)
Part 1 of a set of changes for making the recording of highlights and "Play of the Game"s to mp4 in Overwatch work. This MR contains most of the required IMFMediaSink changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 6c8d69f73df..cd41d40000b 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -91,6 +91,7 @@ struct media_sink IMFByteStream *bytestream; IMFMediaEventQueue *event_queue; + IMFPresentationClock *clock; struct list stream_sinks; @@ -877,9 +878,31 @@ static HRESULT WINAPI media_sink_GetStreamSinkById(IMFFinalizableMediaSink *ifac static HRESULT WINAPI media_sink_SetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock *clock) { - FIXME("iface %p, clock %p stub!\n", iface, clock); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); + HRESULT hr = S_OK; - return E_NOTIMPL; + TRACE("iface %p, clock %p\n", iface, clock); + + if (media_sink->clock) + { + hr = IMFPresentationClock_RemoveClockStateSink(media_sink->clock, &media_sink->IMFClockStateSink_iface); + if (!SUCCEEDED(hr)) + goto done; + IMFPresentationClock_Release(media_sink->clock); + media_sink->clock = NULL; + } + + if (clock) + { + hr = IMFPresentationClock_AddClockStateSink(clock, &media_sink->IMFClockStateSink_iface); + if (!SUCCEEDED(hr)) + goto done; + IMFPresentationClock_AddRef(clock); + media_sink->clock = clock; + } + +done: + return hr; } static HRESULT WINAPI media_sink_GetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock **clock) @@ -913,6 +936,11 @@ static HRESULT WINAPI media_sink_Shutdown(IMFFinalizableMediaSink *iface) IMFMediaEventQueue_Shutdown(media_sink->event_queue); IMFByteStream_Close(media_sink->bytestream); + if (media_sink->clock) + { + IMFPresentationClock_RemoveClockStateSink(media_sink->clock, &media_sink->IMFClockStateSink_iface); + IMFPresentationClock_Release(media_sink->clock); + } media_sink->state = STATE_SHUTDOWN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index cd41d40000b..ebabf4c1eae 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -907,9 +907,15 @@ done: static HRESULT WINAPI media_sink_GetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock **clock) { - FIXME("iface %p, clock %p stub!\n", iface, clock); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); - return E_NOTIMPL; + TRACE("iface %p, clock %p stub!\n", iface, clock); + + if (!media_sink->clock) + return MF_E_NO_CLOCK; + + *clock = media_sink->clock; + return S_OK; } static HRESULT WINAPI media_sink_Shutdown(IMFFinalizableMediaSink *iface) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/media_sink.c | 28 +++++++++-- dlls/winegstreamer/mfplat.c | 81 ++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 4 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 2374fb6fddd..07a02e42cc3 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -126,6 +126,7 @@ extern HRESULT mfplat_DllRegisterServer(void); IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format); void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format); +HRESULT mf_media_type_add_aac_user_data(IMFMediaType *type); HRESULT wg_sample_create_mf(IMFSample *sample, struct wg_sample **out); HRESULT wg_sample_create_quartz(IMediaSample *sample, struct wg_sample **out); diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index ebabf4c1eae..128f103f7bb 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -757,26 +757,44 @@ static HRESULT WINAPI media_sink_AddStreamSink(IMFFinalizableMediaSink *iface, D IMFMediaType *media_type, IMFStreamSink **stream_sink) { struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); + IMFMediaType *type_copy = NULL; struct stream_sink *object; + GUID major_type, subtype; struct wg_format format; HRESULT hr; TRACE("iface %p, stream_sink_id %#lx, media_type %p, stream_sink %p.\n", iface, stream_sink_id, media_type, stream_sink); + if (!FAILED(IMFMediaType_GetMajorType(media_type, &major_type)) && + IsEqualGUID(&major_type, &MFMediaType_Audio) && + !FAILED(IMFMediaType_GetGUID(media_type, &MF_MT_SUBTYPE, &subtype)) && + (IsEqualGUID(&subtype, &MFAudioFormat_AAC) || IsEqualGUID(&subtype, &MFAudioFormat_RAW_AAC)) && + FAILED(IMFMediaType_GetItem(media_type, &MF_MT_USER_DATA, NULL))) + { + if (FAILED(hr = MFCreateMediaType(&type_copy)) || + FAILED(hr = IMFMediaType_CopyAllItems(media_type, (IMFAttributes*)type_copy))) + goto done; + hr = mf_media_type_add_aac_user_data(type_copy); + if (FAILED(hr)) + goto done; + media_type = type_copy; + } + EnterCriticalSection(&media_sink->cs); if (media_sink_get_stream_sink_by_id(media_sink, stream_sink_id)) { LeaveCriticalSection(&media_sink->cs); - return MF_E_STREAMSINK_EXISTS; + hr = MF_E_STREAMSINK_EXISTS; + goto done; } if (FAILED(hr = stream_sink_create(stream_sink_id, media_type, media_sink, &object))) { WARN("Failed to create stream sink, hr %#lx.\n", hr); LeaveCriticalSection(&media_sink->cs); - return hr; + goto done; } mf_media_type_to_wg_format(media_type, &format); @@ -784,7 +802,7 @@ static HRESULT WINAPI media_sink_AddStreamSink(IMFFinalizableMediaSink *iface, D { LeaveCriticalSection(&media_sink->cs); IMFStreamSink_Release(&object->IMFStreamSink_iface); - return hr; + goto done; } list_add_tail(&media_sink->stream_sinks, &object->entry); @@ -794,7 +812,9 @@ static HRESULT WINAPI media_sink_AddStreamSink(IMFFinalizableMediaSink *iface, D if (stream_sink) IMFStreamSink_AddRef((*stream_sink = &object->IMFStreamSink_iface)); - return S_OK; +done: + if (type_copy) IMFMediaType_Release(type_copy); + return hr; } static HRESULT WINAPI media_sink_RemoveStreamSink(IMFFinalizableMediaSink *iface, DWORD stream_sink_id) diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index e6d9fb9fd2c..0572b3bfd64 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -25,6 +25,7 @@ #include "initguid.h" #include "d3d9types.h" #include "mfapi.h" +#include "mferror.h" #include "mmreg.h" #include "wine/debug.h" @@ -904,3 +905,83 @@ void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format) else FIXME("Unrecognized major type %s.\n", debugstr_guid(&major_type)); } + +static inline int min_int(int a, int b) +{ + return a < b ? a : b; +} + +static inline void write_bits(BYTE *dst, int *bit_pos, UINT32 val, int bits) +{ + BYTE *start_ptr = dst + *bit_pos / 8; + int used = *bit_pos % 8; + *bit_pos += bits; + + do + { + int c = min_int(8 - used, bits); + BYTE m = ((1U << c) - 1); + BYTE v = val >> (bits - c); + *start_ptr = (*start_ptr & ~(m << (8 - c - used))) | ((v & m) << (8 - c - used)); + bits -= c; + start_ptr++; + used = 0; + } while (bits); +} + +HRESULT mf_media_type_add_aac_user_data(IMFMediaType *type) +{ + BYTE buffer[64]; + HEAACWAVEFORMAT *user_data = (HEAACWAVEFORMAT *)buffer; + UINT32 frequency, channels, payload_type, profile_level, size; + BYTE *ptr = user_data->pbAudioSpecificConfig; + int bits = 0; + + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &frequency)) || + frequency > 0xFFFFFF) + return MF_E_INVALIDMEDIATYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &channels)) || + channels < 1 || channels > 8 || channels == 7) + return MF_E_INVALIDMEDIATYPE; + + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AAC_PAYLOAD_TYPE, &payload_type))) + payload_type = 0; + else if (payload_type > 3) + return MF_E_INVALIDMEDIATYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AAC_AUDIO_PROFILE_LEVEL_INDICATION, &profile_level))) + profile_level = 0x29; + else if (profile_level > 0xFFFF) + return MF_E_INVALIDMEDIATYPE; + + if (channels == 8) + channels = 7; + + memset( user_data, 0, sizeof(buffer) ); + user_data->wfInfo.wPayloadType = payload_type; + user_data->wfInfo.wAudioProfileLevelIndication = profile_level; + write_bits(ptr, &bits, 2, 5); + switch (frequency) + { + case 96000: write_bits(ptr, &bits, 0, 4); break; + case 88200: write_bits(ptr, &bits, 1, 4); break; + case 64000: write_bits(ptr, &bits, 2, 4); break; + case 48000: write_bits(ptr, &bits, 3, 4); break; + case 44100: write_bits(ptr, &bits, 4, 4); break; + case 32000: write_bits(ptr, &bits, 5, 4); break; + case 24000: write_bits(ptr, &bits, 6, 4); break; + case 22050: write_bits(ptr, &bits, 7, 4); break; + case 16000: write_bits(ptr, &bits, 8, 4); break; + case 12000: write_bits(ptr, &bits, 9, 4); break; + case 11025: write_bits(ptr, &bits, 10, 4); break; + case 8000: write_bits(ptr, &bits, 11, 4); break; + case 7350: write_bits(ptr, &bits, 12, 4); break; + default: + write_bits(ptr, &bits, 15, 4); + write_bits(ptr, &bits, frequency, 24); + break; + } + write_bits(ptr, &bits, channels, 4); + + size = FIELD_OFFSET(HEAACWAVEFORMAT, pbAudioSpecificConfig[(bits + 7) / 8]) - sizeof(user_data->wfInfo.wfx); + return IMFMediaType_SetBlob(type, &MF_MT_USER_DATA, (void*)(&user_data->wfInfo.wfx + 1), size); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 128f103f7bb..5f824033527 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -587,7 +587,12 @@ static HRESULT media_sink_start(struct media_sink *media_sink) media_sink->state = STATE_STARTED; - return media_sink_queue_stream_event(media_sink, MEStreamSinkStarted); + if (FAILED(hr = media_sink_queue_stream_event(media_sink, MEStreamSinkStarted))) + TRACE("Queueing MEStreamSinkStarted failed\n"); + else if (FAILED(hr = media_sink_queue_stream_event(media_sink, MEStreamSinkRequestSample))) + TRACE("Queueing MEStreamSinkRequestSample failed\n"); + + return hr; } static HRESULT media_sink_stop(struct media_sink *media_sink) @@ -605,6 +610,7 @@ static HRESULT media_sink_pause(struct media_sink *media_sink) static HRESULT media_sink_process(struct media_sink *media_sink, IMFSample *sample, UINT32 stream_id) { + struct stream_sink *stream_sink = NULL; wg_muxer_t muxer = media_sink->muxer; struct wg_sample *wg_sample; LONGLONG time, duration; @@ -637,6 +643,18 @@ static HRESULT media_sink_process(struct media_sink *media_sink, IMFSample *samp hr = wg_muxer_push_sample(muxer, wg_sample, stream_id); wg_sample_release(wg_sample); + if (SUCCEEDED(hr)) + { + LIST_FOR_EACH_ENTRY(stream_sink, &media_sink->stream_sinks, struct stream_sink, entry) + { + if (stream_sink->id == stream_id) + { + hr = IMFMediaEventQueue_QueueEventParamVar(stream_sink->event_queue, MEStreamSinkRequestSample, &GUID_NULL, S_OK, NULL); + break; + } + } + } + return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 5f824033527..4ccf9b3bb51 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -446,6 +446,8 @@ static HRESULT WINAPI stream_sink_type_handler_SetCurrentMediaType(IMFMediaTypeH static HRESULT WINAPI stream_sink_type_handler_GetCurrentMediaType(IMFMediaTypeHandler *iface, IMFMediaType **type) { struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface); + IMFMediaType *type_copy = NULL; + HRESULT hr; TRACE("iface %p, type %p.\n", iface, type); @@ -454,9 +456,15 @@ static HRESULT WINAPI stream_sink_type_handler_GetCurrentMediaType(IMFMediaTypeH if (!stream_sink->type) return MF_E_NOT_INITIALIZED; - IMFMediaType_AddRef((*type = stream_sink->type)); - - return S_OK; + hr = MFCreateMediaType(&type_copy); + if (FAILED(hr)) + return hr; + hr = IMFMediaType_CopyAllItems(stream_sink->type, (IMFAttributes*)type_copy); + if (FAILED(hr)) + IMFMediaType_Release(type_copy); + else + *type = type_copy; + return hr; } static HRESULT WINAPI stream_sink_type_handler_GetMajorType(IMFMediaTypeHandler *iface, GUID *type) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
From: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 4ccf9b3bb51..fd734d9e739 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -469,9 +469,18 @@ static HRESULT WINAPI stream_sink_type_handler_GetCurrentMediaType(IMFMediaTypeH static HRESULT WINAPI stream_sink_type_handler_GetMajorType(IMFMediaTypeHandler *iface, GUID *type) { - FIXME("iface %p, type %p.\n", iface, type); + struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(stream_sink->media_sink); + HRESULT hr = MF_E_ATTRIBUTENOTFOUND; - return E_NOTIMPL; + TRACE("iface %p, type %p.\n", iface, type); + + EnterCriticalSection(&media_sink->cs); + if (stream_sink->type) + hr = IMFMediaType_GetGUID(stream_sink->type, &MF_MT_MAJOR_TYPE, type); + LeaveCriticalSection(&media_sink->cs); + + return hr; } static const IMFMediaTypeHandlerVtbl stream_sink_type_handler_vtbl = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5159
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=143412 Your paranoid android. === debian11b (64 bit WoW report) === mf: mf.c:7802: Test succeeded inside todo block: Unexpected hr 0. mf.c:7806: Test succeeded inside todo block: Unexpected hr 0. Unhandled exception: page fault on write access to 0x0000000000000008 in 64-bit code (0x00000074757597). Report validation errors: mf:mf prints too much data (35863 bytes)
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
static HRESULT WINAPI media_sink_SetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock *clock) { - FIXME("iface %p, clock %p stub!\n", iface, clock); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); + HRESULT hr = S_OK;
- return E_NOTIMPL; + TRACE("iface %p, clock %p\n", iface, clock); + + if (media_sink->clock)
You probably need to lock cs around this, which would make the done label actually useful. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62621
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
static HRESULT WINAPI media_sink_SetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock *clock) { - FIXME("iface %p, clock %p stub!\n", iface, clock); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); + HRESULT hr = S_OK;
- return E_NOTIMPL; + TRACE("iface %p, clock %p\n", iface, clock); + + if (media_sink->clock) + { + hr = IMFPresentationClock_RemoveClockStateSink(media_sink->clock, &media_sink->IMFClockStateSink_iface); + if (!SUCCEEDED(hr))
```suggestion:-0+0 if (FAILED(hr)) ``` Same everywhere else. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62622
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
static HRESULT WINAPI media_sink_GetPresentationClock(IMFFinalizableMediaSink *iface, IMFPresentationClock **clock) { - FIXME("iface %p, clock %p stub!\n", iface, clock); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface);
- return E_NOTIMPL; + TRACE("iface %p, clock %p stub!\n", iface, clock); + + if (!media_sink->clock) + return MF_E_NO_CLOCK; + + *clock = media_sink->clock; + return S_OK;
Same thing, probably needs a lock. Also needs to AddRef on the returned clock. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62623
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
iface, stream_sink_id, media_type, stream_sink);
+ if (!FAILED(IMFMediaType_GetMajorType(media_type, &major_type)) && + IsEqualGUID(&major_type, &MFMediaType_Audio) && + !FAILED(IMFMediaType_GetGUID(media_type, &MF_MT_SUBTYPE, &subtype)) && + (IsEqualGUID(&subtype, &MFAudioFormat_AAC) || IsEqualGUID(&subtype, &MFAudioFormat_RAW_AAC)) && + FAILED(IMFMediaType_GetItem(media_type, &MF_MT_USER_DATA, NULL))) + { + if (FAILED(hr = MFCreateMediaType(&type_copy)) || + FAILED(hr = IMFMediaType_CopyAllItems(media_type, (IMFAttributes*)type_copy))) + goto done; + hr = mf_media_type_add_aac_user_data(type_copy); + if (FAILED(hr)) + goto done; + media_type = type_copy; + }
Doesn't seem right to have to do that here. Please 1) a test to check that not passing MF_MT_USER_DATA works, and 2) probably this done elsewhere, for instance fallback to individual AAC attributes in `mf_media_type_to_wg_format_audio_mpeg4` when MF_MT_USER_DATA is missing. (Also `!FAILED` should be `SUCCEEDED`) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62624
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
media_sink->state = STATE_STARTED;
- return media_sink_queue_stream_event(media_sink, MEStreamSinkStarted); + if (FAILED(hr = media_sink_queue_stream_event(media_sink, MEStreamSinkStarted))) + TRACE("Queueing MEStreamSinkStarted failed\n"); + else if (FAILED(hr = media_sink_queue_stream_event(media_sink, MEStreamSinkRequestSample))) + TRACE("Queueing MEStreamSinkRequestSample failed\n");
Would be nice to have tests to validate that these events are supposed to be queued. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62625
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
hr = wg_muxer_push_sample(muxer, wg_sample, stream_id); wg_sample_release(wg_sample);
+ if (SUCCEEDED(hr)) + { + LIST_FOR_EACH_ENTRY(stream_sink, &media_sink->stream_sinks, struct stream_sink, entry) + { + if (stream_sink->id == stream_id) + { + hr = IMFMediaEventQueue_QueueEventParamVar(stream_sink->event_queue, MEStreamSinkRequestSample, &GUID_NULL, S_OK, NULL);
Same thing for this, would be nice to have tests to check that new sample is automatically requested after one is processed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62626
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
if (!stream_sink->type) return MF_E_NOT_INITIALIZED;
- IMFMediaType_AddRef((*type = stream_sink->type)); - - return S_OK; + hr = MFCreateMediaType(&type_copy); + if (FAILED(hr)) + return hr; + hr = IMFMediaType_CopyAllItems(stream_sink->type, (IMFAttributes*)type_copy); + if (FAILED(hr)) + IMFMediaType_Release(type_copy); + else + *type = type_copy;
```suggestion:-7+0 if (FAILED(hr = MFCreateMediaType(&type_copy))) return hr; if (FAILED(hr = IMFMediaType_CopyAllItems(stream_sink->type, (IMFAttributes*)type_copy))) IMFMediaType_Release(type_copy); else *type = type_copy; ``` Nit: lets save some lines. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62627
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
static HRESULT WINAPI stream_sink_type_handler_GetMajorType(IMFMediaTypeHandler *iface, GUID *type) { - FIXME("iface %p, type %p.\n", iface, type); + struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(stream_sink->media_sink); + HRESULT hr = MF_E_ATTRIBUTENOTFOUND;
- return E_NOTIMPL; + TRACE("iface %p, type %p.\n", iface, type); + + EnterCriticalSection(&media_sink->cs); + if (stream_sink->type) + hr = IMFMediaType_GetGUID(stream_sink->type, &MF_MT_MAJOR_TYPE, type); + LeaveCriticalSection(&media_sink->cs);
You don't need to lock the cs here, for now at least, as stream_sink->type is never written to after creation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62628
You're so fast at reviewing, I just submitted this MR! Thank you for the feedback, I will look into it and adjust the MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62631
On Mon Feb 26 10:34:46 2024 +0000, Rémi Bernon wrote:
```suggestion:-0+0 if (FAILED(hr)) ``` Same everywhere else.
Ha, yeah, it's just programmer brain going "What's the opposite of SUCCEEDED? Right, \!SUCCEEDED" -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5159#note_62633
participants (4)
-
Marvin -
Rémi Bernon -
Torge Matthies
-
Torge Matthies (＠tmatthies)