[FFmpeg-devel] [PATCH 23/26] lavc/cbs: Add JPEG support

[Mark Thompson]

On 25/04/18 23:23, James Almer wrote:
> On 4/25/2018 6:31 PM, Mark Thompson wrote:
>> +static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
>> +                                   CodedBitstreamFragment *frag,
>> +                                   int header)
>> +{
>> +    AVBufferRef *data_ref;
>> +    uint8_t *data;
>> +    size_t data_size;
>> +    int unit, start, end, marker, next_start, next_marker;
>> +    int err, i, j, length;
>> +
>> +    if (frag->data_size < 4) {
>> +        // Definitely too short to be meaningful.
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    for (i = 0; i + 1 < frag->data_size && frag->data[i] != 0xff; i++);
>> +    if (i > 0) {
>> +        av_log(ctx->log_ctx, AV_LOG_WARNING, "Discarding %d bytes at "
>> +               "beginning of image.\n", i);
>> +    }
>> +    for (++i; i + 1 < frag->data_size && frag->data[i] == 0xff; i++);
>> +    if (i + 1 >= frag->data_size && frag->data[i]) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
>> +               "no SOI marker found.\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    marker = frag->data[i];
>> +    if (marker != JPEG_MARKER_SOI) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: first "
>> +               "marker is %02x, should be SOI.\n", marker);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    for (++i; i + 1 < frag->data_size && frag->data[i] == 0xff; i++);
>> +    if (i + 1 >= frag->data_size) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
>> +               "no image content found.\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    marker = frag->data[i];
>> +    start  = i + 1;
>> +
>> +    for (unit = 0;; unit++) {
>> +        if (marker == JPEG_MARKER_EOI) {
>> +            break;
>> +        } else if (marker == JPEG_MARKER_SOS) {
>> +            for (i = start; i + 1 < frag->data_size; i++) {
>> +                if (frag->data[i] != 0xff)
>> +                    continue;
>> +                end = i;
>> +                for (++i; i + 1 < frag->data_size &&
>> +                          frag->data[i] == 0xff; i++);
>> +                if (i + 1 >= frag->data_size) {
>> +                    next_marker = -1;
>> +                } else {
>> +                    if (frag->data[i] == 0x00)
>> +                        continue;
>> +                    next_marker = frag->data[i];
>> +                    next_start  = i + 1;
>> +                }
>> +                break;
>> +            }
>> +        } else {
>> +            i = start;
>> +            if (i + 2 > frag->data_size) {
>> +                av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
>> +                       "truncated at %02x marker.\n", marker);
>> +                return AVERROR_INVALIDDATA;
>> +            }
>> +            length = AV_RB16(frag->data + i);
>> +            if (i + length > frag->data_size) {
>> +                av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
>> +                       "truncated at %02x marker segment.\n", marker);
>> +                return AVERROR_INVALIDDATA;
>> +            }
>> +            end = start + length;
>> +
>> +            i = end;
>> +            if (frag->data[i] != 0xff) {
>> +                next_marker = -1;
>> +            } else {
>> +                for (++i; i + 1 < frag->data_size &&
>> +                          frag->data[i] == 0xff; i++);
>> +                if (i + 1 >= frag->data_size) {
>> +                    next_marker = -1;
>> +                } else {
>> +                    next_marker = frag->data[i];
>> +                    next_start  = i + 1;
>> +                }
>> +            }
>> +        }
>> +
>> +        if (marker == JPEG_MARKER_SOS) {
>> +            length = AV_RB16(frag->data + start);
>> +
>> +            data_ref = av_buffer_alloc(end - start +
>> +                                       AV_INPUT_BUFFER_PADDING_SIZE);
> 
> Allocating an AVBufferRef here, letting ff_cbs_insert_unit_data()
> generate a new reference to it, then unreffing the first one seems
> wasteful/redundant.
> 
> Use av_malloc(), set data_ref to NULL, and let ff_cbs_insert_unit_data()
> create the AVBufferRef by taking ownership of the malloc'd data array
> instead. You'd then only need to free said data array if
> ff_cbs_insert_unit_data() fails.

Right, yes.  Changed locally.

Thanks,

- Mark