[PATCH v4 3/3] winegstreamer: Implement IMFMediaStream::GetStreamDescriptor.
Zebediah Figura
zfigura at codeweavers.com
Thu Sep 24 18:42:34 CDT 2020
On 9/24/20 3:56 PM, Derek Lesho wrote:
>
> On 9/24/20 12:27 PM, Zebediah Figura wrote:
>>> enum
>>> {
>>> STREAM_STUB,
>>> + STREAM_INACTIVE,
>> What's the difference between these two states? There doesn't seem to be
>> a difference in this patch.
> I think it's just a vestige of a prior version of the patchset, I'll
> merge STREAM_STUB into STREAM_INACTIVE.
Are there other members of this enumeration that will be introduced, or
can it become a simple boolean flag?
>>
>> +static HRESULT media_stream_init_desc(struct media_stream *stream)
>> +{
>> + GstCaps *current_caps = gst_pad_get_current_caps(stream->their_src);
>> + IMFMediaTypeHandler *type_handler;
>> + IMFMediaType *stream_type = NULL;
>> + HRESULT hr;
>> +
>> + if (!current_caps)
>> + {
>> + hr = E_FAIL;
>> + goto fail;
>> + }
>> Can that happen?
> I don't think so, I'll remove it since you don't think so either.
>>
>> Note that this "goto" and a couple others below could be replaced with
>> "return hr". In general I'm inclined to think goto isn't worthwhile when
>> there's only one line of cleanup.
> I agree, however the cleanup gets more advanced in future patches. Shall
> I just defer the label cleanup until then?
Probably, though if it's too much of a hassle don't worry about it.
>>> + g_signal_emit_by_name(object->streams[i]->appsink, "pull-preroll", &preroll);
>> Calling gst_app_sink_pull_preroll() directly seems a little less
>> confusing (i.e. in the sense that I wouldn't have had to look up how
>> G_SIGNAL_ACTION works).
> Doesn't this require linking the libgstapp library? Seems like a lot of
> effort to go through to make a few lines prettier.
>
>
Yes, you're right, in that case it's probably best to leave it as is.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200924/6c6d6bf9/attachment-0001.sig>
More information about the wine-devel
mailing list