Summary: | [GStreamer][GTK] Add GRefPtr::outPtr() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brendan Long <b.long> | ||||||
Component: | Platform | Assignee: | Brendan Long <b.long> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue, danw, eric.carlson, glenn, gustavo, jer.noble, menard, mrobinson, pnormand, rakuco | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Brendan Long
2013-10-17 14:39:25 PDT
Created attachment 214516 [details]
Patch
Actually, I missed a couple places, and this one makes me wonder if outPtr() should just deref any existing m_ptr: void MediaPlayerPrivateGStreamer::updateAudioSink() { if (!m_playBin) return; GstElement* sinkPtr = 0; g_object_get(m_playBin.get(), "audio-sink", &sinkPtr, NULL); m_webkitAudioSink = adoptGRef(sinkPtr); } With my current patch, I could do: void MediaPlayerPrivateGStreamer::updateAudioSink() { if (!m_playBin) return; m_webkitAudioSink.clear(); g_object_get(m_playBin.get(), "audio-sink", &m_webkitAudioSink.outPtr(), NULL); } But if I changed outPtr to this: T*& outPtr() { if (m_ptr) { derefGPtr(ptr); m_ptr = 0; /* to prevent incorrect usage */ } return m_ptr; } I could do: void MediaPlayerPrivateGStreamer::updateAudioSink() { if (!m_playBin) return; g_object_get(m_playBin.get(), "audio-sink", &m_webkitAudioSink.outPtr(), NULL); } Created attachment 214530 [details]
Patch
I added more cases where this is useful (previously I searched for _unref, this time I searched for g_object_get).
Oh, and I didn't change outPtr(), since I think it's valuable for it to have the same semantics as GOwnPtr::outPtr(). Comment on attachment 214530 [details]
Patch
Neat!
Comment on attachment 214530 [details] Patch Clearing flags on attachment: 214530 Committed r157624: <http://trac.webkit.org/changeset/157624> All reviewed patches have been landed. Closing bug. At some point it may be useful to make outPtr() implicitly clear() the internal pointer, since it would also make any usage of strings much nicer, letting change this: GstTagList* tags = 0; gst_event_parse_tag(event.get(), &tags); ASSERT(tags); gchar* tagValue; if (gst_tag_list_get_string(tags, GST_TAG_TITLE, &tagValue)) { INFO_MEDIA_MESSAGE("Audio track %d got title %s.", m_index, tagValue); label = tagValue; g_free(tagValue); } if (gst_tag_list_get_string(tags, GST_TAG_LANGUAGE_CODE, &tagValue)) { INFO_MEDIA_MESSAGE("Audio track %d got language %s.", m_index, tagValue); language = tagValue; g_free(tagValue); } To this: GstTagList* tags = 0; gst_event_parse_tag(event.get(), &tags); ASSERT(tags); GOwnPtr<gchar> tagValue; if (gst_tag_list_get_string(tags, GST_TAG_TITLE, &tagValue.outPtr())) { INFO_MEDIA_MESSAGE("Audio track %d got title %s.", m_index, tagValue); label = tagValue; } if (gst_tag_list_get_string(tags, GST_TAG_LANGUAGE_CODE, &tagValue.outPtr())) { INFO_MEDIA_MESSAGE("Audio track %d got language %s.", m_index, tagValue); language = tagValue; } |