[Webkit-unassigned] [Bug 180080] [EME][GStreamer] Add the full-sample encryption support in the GStreamer ClearKey decryptor

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=180080

Xabier Rodríguez Calvar <calvaris at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #327749|commit-queue?               |commit-queue-
              Flags|                            |

--- Comment #2 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 327749
  --> https://bugs.webkit.org/attachment.cgi?id=327749
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327749&action=review

I think given the complexity of this algorithm and considering that we are inside a GStreamer element we can do some GStreamer-like coding. In this case, I think GStreamer gotos are a good option. In the subsamples section we have "park" that is going to free the reader and the subsamples map, it is going to fallback to beach after the if clause finishes and if is going to free the buffer map. In the whole buffer decryption we just fallback to beach.

This is not an r+ because it is not ready IMHO but it is not an r- either because technically I see nothing wrong.

What would be really nice would be to have something like UniquePtr but for this kind of stack variables that need some action to free them. Anyway I think it would be out of the scope of this bug unless you feel like doing it.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:238
> +    if (subSampleCount) {

Before this:

bool returnValue = true;

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:242
> +        gboolean subsamplesBufferMapped = gst_buffer_map(subSamplesBuffer, &subSamplesMap, GST_MAP_READ);

gboolean -> bool

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:246
> +            gst_buffer_unmap(buffer, &map);
> +            return false;

returnValue = false;
goto beach;

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:251
> +        unsigned sampleIndex = 0;

Move sampleIndex to below GST_DEBUG_OBJECT.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:257
> +            guint16 nBytesClear = 0;
> +            guint32 nBytesEncrypted = 0;

uint16_t and uint32_t

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:266
> +                    gst_byte_reader_free(reader);
> +                    gst_buffer_unmap(buffer, &map);
> +                    gst_buffer_unmap(subSamplesBuffer, &subSamplesMap);
> +                    return false;

returnValue = false;
goto park;

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:285
> +                    gst_byte_reader_free(reader);
> +                    gst_buffer_unmap(buffer, &map);
> +                    gst_buffer_unmap(subSamplesBuffer, &subSamplesMap);
> +                    return false;

ditto

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:291
> +        gst_byte_reader_free(reader);
> +        gst_buffer_unmap(subSamplesBuffer, &subSamplesMap);

"park:" here

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:296
> +        guint32 nBytesEncrypted = map.size;

You don't need this variable

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:305
>                  gst_buffer_unmap(buffer, &map);
> -                gst_buffer_unmap(subSamplesBuffer, &subSamplesMap);
>                  return false;

returnValue = false;
goto beach;

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:309
>      gst_buffer_unmap(buffer, &map);

beach: here

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171129/c4a06369/attachment-0001.html>