[vlc-devel] [PATCH 2/2] Add support for the new Android JellyBean MediaCodec API

[Jean-Baptiste Kempf]

On Fri, Oct 05, 2012 at 09:45:42PM +0300, Martin Storsjö wrote :
> +    int num_codecs, i;

You know you can use C99, right? ;)

> +    switch (p_dec->fmt_in.i_codec) {
> +    case VLC_CODEC_H264: mime = "video/avc"; break;
> +    case VLC_CODEC_H263: mime = "video/3gpp"; break;
> +    case VLC_CODEC_MP4V: mime = "video/mp4v-es"; break;
> +    default:
> +        msg_Err(p_dec, "codec %d not supported", p_dec->fmt_in.i_codec);

Why a msg_Err, here?

> +    msg_Dbg(p_dec, "%d codecs", num_codecs);

Maybe a more explicit message here?

> +        int num_types, j, found = 0;

Shouldn't found be a bool ? OK, it does not matter ;)

> +    format = (*env)->CallStaticObjectMethod(env, p_sys->media_format_class, p_sys->create_video_format, (*env)->NewStringUTF(env, mime), p_dec->fmt_in.video.i_width, p_dec->fmt_in.video.i_height);

Break the long line?


> +		// TODO: Use crop_top/crop_left as well? Or is that already taken into account? Shouldn't be done on OMX_TI_COLOR_FormatYUV420PackedSemiPlanar at least.

No tabs, please.

> +                p_pic->date = (*env)->GetLongField(env, p_sys->buffer_info, p_sys->pts_field);
> +                GetVlcChromaSizes(p_dec->fmt_out.i_codec, p_dec->fmt_out.video.i_width, p_dec->fmt_out.video.i_height, NULL, NULL, &chroma_div);
> +                CopyOmxPicture(p_sys->pixel_format, p_pic, p_sys->slice_height, p_sys->stride, ptr, chroma_div);


> +        } else if (index == -3) { // INFO_OUTPUT_BUFFERS_CHANGED

Should this be #define'd or in an enum?

> +        memcpy(bufptr, p_block->p_buffer, size);

<mode=av500> memcpy? there is a bug. :) </mode>

> +        if (p_sys->nal_size) {
> +            int len = size;
> +            uint8_t *ptr = bufptr;
> +            while (len >= p_sys->nal_size) {
> +                uint32_t i, nal_len = 0;
> +                for (i = 0; i < p_sys->nal_size; i++) {
> +                    nal_len = (nal_len << 8) | ptr[i];
> +                    ptr[i] = 0;
> +                }
> +                ptr[p_sys->nal_size - 1] = 1;
> +                if (nal_len > INT_MAX || nal_len > (unsigned int) len)
> +                    break;
> +                ptr += nal_len + p_sys->nal_size;
> +                len -= nal_len + p_sys->nal_size;
> +            }
> +        }

Should this be splitted for reuse?


In general, not much to say :)

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device