[FFmpeg-devel] [PATCH 1/1] Add Sega FILM muxer

[Josh de Kock]

On 2018/04/02 3:56, misty at brew.sh wrote:
> From: Misty De Meo <mistydemeo at gmail.com>
> 
> [...]
> --- /dev/null
> +++ b/libavformat/segafilmenc.c
> @@ -0,0 +1,397 @@
> +/*
> + * Sega FILM Format (CPK) Muxer
> + * Copyright (C) 2003 The FFmpeg project
I assume you copied the demuxer but shouldn't you be here as well?

> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> [...]
> +    codec_id = format_context->streams[pkt->stream_index]->codecpar->codec_id;
> +
> +    /* Sega Cinepak has an extra two-byte header; write dummy data there,
> +     * then adjust the cvid header to accommodate for the extra size */
> +    if (codec_id == AV_CODEC_ID_CINEPAK) {
> +        encoded_buf_size = AV_RB24(&pkt->data[1]);
> +        // Already Sega Cinepak, so no need to reformat the packets
> +        if (encoded_buf_size != pkt->size && (pkt->size % encoded_buf_size) != 0) {
> +            avio_write(pb, pkt->data, pkt->size);
> +        } else {
> +            /* In Sega Cinepak, the reported size in the Cinepak header is
> +             * 8 bytes too short. However, the size in the STAB section of the header
> +             * is correct, taking into account the extra two bytes. */
I honestly think your comment style (i.e. C vs C++) should be consistent 
throughout the file, but that's just me.

> [...]

Generally, I'm ok with this patch. I don't think we're losing much by 
adding a small muxer for a format we already have a demuxer for. If it 
were some insanely complex muxer then I might have a different opinion.

-- 
Josh