Review Request 121638: Make VideoWidget::snapshot work again

[Harald Sitter]

> On Dec. 23, 2014, 1:38 a.m., Rohan Garg wrote:
> > Looks sensible to me, unless someone can come up with a better reason not to do this ( I couldn't find documentation anywhere about GST_VIDEO_NE, except for inside gst code itself, and no one, not even gst seems to be using this. )
> 
> Harald Sitter wrote:
>     But but but you used it? :P 
>     http://lists.kde.org/?l=kde-commits&m=136482135607986
>     
>     GST_VIDEO_NE supposedly stands for native endian giving the native endian type "RGB_LE" or "RGB_BE" depending on platform.
>     
>     I do wonder if that line actually makes sense though... the original gstreamer0.10 version was:
>     +                gst_caps_new_simple("video/x-raw-rgb",
>     +                                    "bpp", G_TYPE_INT, 24,
>     +                                    "depth", G_TYPE_INT, 24,
>     +                                    "endianness", G_TYPE_INT, G_BIG_ENDIAN,
>     +                                    "red_mask", G_TYPE_INT, 0xff0000,
>     +                                    "green_mask", G_TYPE_INT, 0x00ff00,
>     +                                    "blue_mask", G_TYPE_INT, 0x0000ff,
>     +                                    NULL);
>     which is a far way from the code we have now. In particular the lack of bits per peixel being defined doesn't make me too excitied about the new version. The buffer of that element is later loaded into a QImage RGB24 (888) so there is really a very strict requirement on the output bpp.
>     
>     Anyway, I'd feel more comfortable with Dan having a look and sprinkling us with wisdom. The blue tint issue a while ago also had to do with bad endianess.

Oh, and, all that being said... I am fine with the review, someone just needs to make sure that Dan actually looks at the snapshot code in general ^^

slightly less broken is still better than entirely broken I say.


- Harald


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121638/#review72444
-----------------------------------------------------------


On Dec. 23, 2014, 1:28 a.m., Albert Astals Cid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121638/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2014, 1:28 a.m.)
> 
> 
> Review request for Phonon, Phonon Backends and Rohan Garg.
> 
> 
> Repository: phonon-gstreamer
> 
> 
> Description
> -------
> 
> I have no clue about gstreamer, but this works and the other not :D
> 
> 
> Diffs
> -----
> 
>   gstreamer/videowidget.cpp 5dd35cb 
> 
> Diff: https://git.reviewboard.kde.org/r/121638/diff/
> 
> 
> Testing
> -------
> 
> https://bugs.kde.org/show_bug.cgi?id=342140 works again
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20141223/3eca5c3c/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia