[Spice-devel] [spice-gtk v3] gstreamer: Take into account stride information

[Uri Lublin]

On 07/25/2017 06:30 PM, Pavel Grunt wrote:
> On Tue, 2017-07-25 at 16:20 +0100, Frediano Ziglio wrote:
>> Using hardware encoders/decoders is possible that the output
>> stride of the image cannot be computed with a fixed formula
>> (width * 4). GStreamer in this case should set GstVideoMeta
>> information with the correct stride value.
>> Consider this value if present.
>> This fix a problem using NvEnc encoder and Intel decoder.
>>
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>> ---
>>   src/channel-display-gst.c   | 9 ++++++++-
>>   src/channel-display-mjpeg.c | 2 +-
>>   src/channel-display-priv.h  | 3 ++-
>>   src/channel-display.c       | 6 ++++--
>>   4 files changed, 15 insertions(+), 5 deletions(-)
>>
>> Changes since v2:
>> - use spice_ prefix for new function.
>>
>> Changes since v1:
>> - use a mnemonic value for unknown stride;
>> - better check for stride in GstVideoMeta.
>>
>> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
>> index 03c6044..338e7d2 100644
>> --- a/src/channel-display-gst.c
>> +++ b/src/channel-display-gst.c
>> @@ -26,6 +26,7 @@
>>   #include <gst/gst.h>
>>   #include <gst/app/gstappsrc.h>
>>   #include <gst/app/gstappsink.h>
>> +#include <gst/video/gstvideometa.h>
>>   
>>   
>>   /* GStreamer decoder implementation */
>> @@ -102,6 +103,12 @@ static void free_gst_frame(SpiceGstFrame *gstframe)
>>   
>>   static void schedule_frame(SpiceGstDecoder *decoder);
>>   
>> +static int spice_gst_buffer_get_stride(GstBuffer *buffer)
>> +{
>> +    GstVideoMeta *video = gst_buffer_get_video_meta(buffer);
>> +    return video && video->n_planes > 0 ? video->stride[0] :
>> SPICE_UNKNOWN_STRIDE;
>> +}
>> +
>>   /* main context */
>>   static gboolean display_frame(gpointer video_decoder)
>>   {
>> @@ -145,7 +152,7 @@ static gboolean display_frame(gpointer video_decoder)
>>       }
>>   
>>       stream_display_frame(decoder->base.stream, gstframe->frame,
>> -                         width, height, mapinfo.data);
>> +                         width, height, spice_gst_buffer_get_stride(buffer),
>> mapinfo.data);
>>       gst_buffer_unmap(buffer, &mapinfo);
>>   
>>    error:
>> diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
>> index 3ba098c..563dc1b 100644
>> --- a/src/channel-display-mjpeg.c
>> +++ b/src/channel-display-mjpeg.c
>> @@ -176,7 +176,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer
>> video_decoder)
>>   
>>       /* Display the frame and dispose of it */
>>       stream_display_frame(decoder->base.stream, decoder->cur_frame,
>> -                         width, height, decoder->out_frame);
>> +                         width, height, SPICE_UNKNOWN_STRIDE, decoder-
>>> out_frame);
>>       free_spice_frame(decoder->cur_frame);
>>       decoder->cur_frame = NULL;
>>       decoder->timer_id = 0;
>> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
>> index 04cb4d1..a0a420c 100644
>> --- a/src/channel-display-priv.h
>> +++ b/src/channel-display-priv.h
>> @@ -192,7 +192,8 @@ G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <=
>> SPICE_VIDEO_CODEC_TYPE_ENUM_END);
>>   
>>   guint32 stream_get_time(display_stream *st);
>>   void stream_dropped_frame_on_playback(display_stream *st);
>> -void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t
>> width, uint32_t height, uint8_t* data);
>> +#define SPICE_UNKNOWN_STRIDE 0
>> +void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t
>> width, uint32_t height, int stride, uint8_t* data);
>>   gint64 get_stream_id_by_stream(SpiceChannel *channel, display_stream *st);
>>   
>>   
>> diff --git a/src/channel-display.c b/src/channel-display.c
>> index 06ed18a..45742a6 100644
>> --- a/src/channel-display.c
>> +++ b/src/channel-display.c
>> @@ -1312,9 +1312,11 @@ void stream_dropped_frame_on_playback(display_stream
>> *st)
>>   /* main context */
>>   G_GNUC_INTERNAL
>>   void stream_display_frame(display_stream *st, SpiceFrame *frame,
>> -                          uint32_t width, uint32_t height, uint8_t *data)
>> +                          uint32_t width, uint32_t height, int stride,
>> uint8_t *data)
>>   {
>> -    int stride = width * sizeof(uint32_t);
>> +    if (stride == SPICE_UNKNOWN_STRIDE) {
>> +        stride = width * sizeof(uint32_t);
>> +    }
>>       if (!(st->flags & SPICE_STREAM_FLAGS_TOP_DOWN)) {
>>           data += stride * (height - 1);
>>           stride = -stride;
> 
> Looks good to me, tested

Looks good to me too. I also tested this.
Thanks,
    Uri.

> 
> Acked-by: Pavel Grunt <pgrunt at redhat.com>
> 
> Thanks,
> Pavel
> 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>