GNOME Bugzilla – Bug 748529
adaptivedemux: only update stream position if query success
Last modified: 2015-05-06 10:41:33 UTC
We don't need to check if result of peer query fail. because it is update to segment position, it's not necessary.
Created attachment 302454 [details] [review] adaptivedemux: only update stream position if query sucess We don't need to check if duration query is fail.
Created attachment 302493 [details] [review] adaptivedemux: only update stream position if query success We don't need to check about the query fail case. because it is update to segment position even though query fail.
Review of attachment 302493 [details] [review]: ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ +1992,2 @@ have_pos = gst_pad_peer_query_position (cur_stream->pad, GST_FORMAT_TIME, Perhaps we can eliminate the have_pos variable altogether and just put the pad query inside the if below. It looks easier to read IMHO.
Thank you for your comment:) I'll update new patch set.
Created attachment 302548 [details] [review] adaptivedemux: only update stream position if query sucess We don't need to check if duration query is fail. So, I update new patch for this case.
(In reply to Thiago Sousa Santos from comment #3) > Review of attachment 302493 [details] [review] [review]: > > ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c > @@ +1992,2 @@ > have_pos = > gst_pad_peer_query_position (cur_stream->pad, GST_FORMAT_TIME, > > Perhaps we can eliminate the have_pos variable altogether and just put the > pad query inside the if below. It looks easier to read IMHO. Could you review my new patch? I upload new patch last week.
Thanks for the update. Pushed. commit 868472affb183f457e3ad584bd8d9dabd8ee5938 Author: Jimmy Ohn <yongjin.ohn@lge.com> Date: Wed Apr 29 18:23:31 2015 +0900 adaptivedemux: only update stream position if query success We don't need to check about the query fail case. because it is update to segment position even though query fail. https://bugzilla.gnome.org/show_bug.cgi?id=748529
(In reply to Thiago Sousa Santos from comment #7) > Thanks for the update. Pushed. commit > 868472affb183f457e3ad584bd8d9dabd8ee5938 Author: Jimmy Ohn > <yongjin.ohn@lge.com> Date: Wed Apr 29 18:23:31 2015 +0900 > adaptivedemux: only update stream position if query success We > don't need to check about the query fail case. because it is update to > segment position even though query fail. > https://bugzilla.gnome.org/show_bug.cgi?id=748529 Thank you thiago:)
(In reply to Thiago Sousa Santos from comment #7) > Thanks for the update. Pushed. commit > 868472affb183f457e3ad584bd8d9dabd8ee5938 Author: Jimmy Ohn > <yongjin.ohn@lge.com> Date: Wed Apr 29 18:23:31 2015 +0900 > adaptivedemux: only update stream position if query success We > don't need to check about the query fail case. because it is update to > segment position even though query fail. > https://bugzilla.gnome.org/show_bug.cgi?id=748529 Could you review my another patch? Actually, I'm wating for review. url is here. https://bugzilla.gnome.org/show_bug.cgi?id=748738