The bytesPerLine is wrong when updating the texture, because the naturalSize() is not updated to the current buffer size, so the stride might be wrong.
Created attachment 204451 [details] Patch
Comment on attachment 204451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204451&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:342 > + GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get()); Would it be better to get the caps before the buffer and pass that as an argument? Is there any way to lock the sink and get the buffer and the caps without the possibility of a race condition? This seems like a pretty serious issue. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:344 > + GRefPtr<GstCaps> caps = GST_BUFFER_CAPS(m_buffer); Any reason you are getting the cap from m_buffer instead of the argument, buffer? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:349 > + int parN, parD, stride; Please don't abbreviate variable names. :)
Comment on attachment 204451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204451&action=review >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:342 >> + GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get()); > > Would it be better to get the caps before the buffer and pass that as an argument? Is there any way to lock the sink and get the buffer and the caps without the possibility of a race condition? This seems like a pretty serious issue. We could held a copy of the current caps in the sink. >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:344 >> + GRefPtr<GstCaps> caps = GST_BUFFER_CAPS(m_buffer); > > Any reason you are getting the cap from m_buffer instead of the argument, buffer? oops! c&p error :$ >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:349 >> + int parN, parD, stride; > > Please don't abbreviate variable names. :) Ok! Thanks!
(In reply to comment #3) > (From update of attachment 204451 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204451&action=review > > >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:342 > >> + GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get()); > > > > Would it be better to get the caps before the buffer and pass that as an argument? Is there any way to lock the sink and get the buffer and the caps without the possibility of a race condition? This seems like a pretty serious issue. > > We could held a copy of the current caps in the sink. Maybe we can do it in this patch?
Created attachment 204460 [details] Patch
Comment on attachment 204451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204451&action=review >>>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:342 >>>> + GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get()); >>> >>> Would it be better to get the caps before the buffer and pass that as an argument? Is there any way to lock the sink and get the buffer and the caps without the possibility of a race condition? This seems like a pretty serious issue. >> >> We could held a copy of the current caps in the sink. > > Maybe we can do it in this patch? I can do it in this patch, though it won't be related with this bug report...
(In reply to comment #6) > I can do it in this patch, though it won't be related with this bug report... I'm not sure I understand, since the patch is introducing a potential race condition. Wouldn't it be better to write the patch without the race condition?
Víctor, when uploading a new patch, please remember to try media/video-display-aspect-ratio.html and if it works, please remove the line at LayoutTests/platform/gtk-wk2/TestExpectations.
Created attachment 204922 [details] Patch
Created attachment 204993 [details] Patch
Comment on attachment 204993 [details] Patch Looks good, thanks!
Comment on attachment 204993 [details] Patch Clearing flags on attachment: 204993 Committed r151733: <http://trac.webkit.org/changeset/151733>
All reviewed patches have been landed. Closing bug.