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

[Misty De Meo]

On Thu, Apr 5, 2018 at 2:06 PM, Josh de Kock  wrote:
> Thanks, pushed. I also clarified with wm4 on IRC that while he was against
> it he wasn't blocking the muxer if someone else pushes it.

Thank you!
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Josh de Kock]

On 2018/04/02 18:53, mi...@brew.sh wrote:

segafilm muxer


Thanks, pushed. I also clarified with wm4 on IRC that while he was 
against it he wasn't blocking the muxer if someone else pushes it.


--
Josh
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Josh de Kock]

On 2018/04/02 3:56, mi...@brew.sh wrote:

From: Misty De Meo 

[...]
--- /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(>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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Wed, Mar 21, 2018 at 10:46:31PM -0700, mi...@brew.sh wrote:
> From: Misty De Meo 
[...]

> +static int film_write_header(AVFormatContext *format_context)
> +{
> +int ret = 0;
> +int64_t stabstart_pos, sample_table_size, stabsize, headersize;
> +int8_t audio_codec;
> +AVIOContext *pb = format_context->pb;
> +FILMOutputContext *film = format_context->priv_data;
> +FILMPacket *prev, *packet;
> +AVStream *audio = NULL;
> +AVStream *video = NULL;
> +
> +/* Calculate how much we need to reserve for the header;
> + * this is the amount the rest of the data will be shifted up by. */
> +sample_table_size = film->packet_count * 16;
> +stabsize = 16 + sample_table_size;
> +headersize = 16 + // FILM header base
> + 32 + // FDSC chunk
> + stabsize;
> +
> +ret = shift_data(format_context, headersize);
> +if (ret < 0)
> +return ret;
> +// Seek back to the beginning to start writing the header now
> +avio_seek(pb, 0, SEEK_SET);
> +
> +if (film->audio_index > -1)
> +audio = format_context->streams[film->audio_index];
> +if (film->video_index > -1)
> +video = format_context->streams[film->video_index];
> +
> +if (audio != NULL) {
> +audio_codec = get_audio_codec_id(audio->codecpar->codec_id);
> +if (audio_codec < 0) {
> +av_log(format_context, AV_LOG_ERROR, "Incompatible audio stream 
> format.\n");
> +return AVERROR(EINVAL);
> +}
> +}
> +
> +// First, write the FILM header; this is very simple
> +
> +ffio_wfourcc(pb, "FILM");
> +avio_wb32(pb, 48 + stabsize);
> +/* This seems to be okay to hardcode, since this muxer targets 1.09 
> features;
> + * videos produced by this muxer are readable by 1.08 and lower players. 
> */
> +ffio_wfourcc(pb, "1.09");
> +// I have no idea what this field does, might be reserved
> +avio_wb32(pb, 0);
> +
> +// Next write the FDSC (file description) chunk
> +ffio_wfourcc(pb, "FDSC");
> +avio_wb32(pb, 0x20); // Size of FDSC chunk
> +// TODO stop hardcoding this if support for another codec is added
> +ffio_wfourcc(pb, "cvid");
> +avio_wb32(pb, video->codecpar->height);
> +avio_wb32(pb, video->codecpar->width);
> +avio_w8(pb, 24); // Bits per pixel - observed to always be 24
> +
> +if (audio != NULL) {
> +avio_w8(pb, audio->codecpar->channels); // Audio channels
> +avio_w8(pb, audio->codecpar->bits_per_coded_sample); // Audio bit 
> depth
> +avio_w8(pb, audio_codec); // Compression - 0 is PCM, 2 is ADX
> +avio_wb16(pb, audio->codecpar->sample_rate); // Audio sampling rate
> +} else {
> +// Set all these fields to 0 if there's no audio
> +avio_w8(pb, 0);
> +avio_w8(pb, 0);
> +avio_w8(pb, 0);
> +avio_wb16(pb, 0);
> +}
> +
> +// I have no idea what this pair of fields does either, might be reserved
> +avio_wb32(pb, 0);
> +avio_wb16(pb, 0);
> +

> +// Finally, write the STAB (sample table) chunk
> +stabstart_pos = avio_tell(pb);

this is set but never used


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Paul B Mahol]

On 3/22/18, mi...@brew.sh  wrote:
> From: Misty De Meo 
>
> ---
>  Changelog |   1 +
>  libavformat/Makefile  |   1 +
>  libavformat/allformats.c  |   1 +
>  libavformat/segafilmenc.c | 380
> ++
>  4 files changed, 383 insertions(+)
>  create mode 100644 libavformat/segafilmenc.c
>
> diff --git a/Changelog b/Changelog
> index 30a8978db4..0ff62ff69d 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -48,6 +48,7 @@ version :
>  - drmeter audio filter
>  - hapqa_extract bitstream filter
>  - filter_units bitstream filter
> +- segafilm muxer
>
>
>  version 3.4:
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 39ec68c28b..4abb992b14 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -462,6 +462,7 @@ OBJS-$(CONFIG_SDR2_DEMUXER)  += sdr2.o
>  OBJS-$(CONFIG_SDS_DEMUXER)   += sdsdec.o
>  OBJS-$(CONFIG_SDX_DEMUXER)   += sdxdec.o
>  OBJS-$(CONFIG_SEGAFILM_DEMUXER)  += segafilm.o
> +OBJS-$(CONFIG_SEGAFILM_MUXER)+= segafilmenc.o
>  OBJS-$(CONFIG_SEGMENT_MUXER) += segment.o
>  OBJS-$(CONFIG_SHORTEN_DEMUXER)   += shortendec.o rawdec.o
>  OBJS-$(CONFIG_SIFF_DEMUXER)  += siff.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 9dc5ce8a76..dfd964f07a 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -364,6 +364,7 @@ extern AVInputFormat  ff_sdr2_demuxer;
>  extern AVInputFormat  ff_sds_demuxer;
>  extern AVInputFormat  ff_sdx_demuxer;
>  extern AVInputFormat  ff_segafilm_demuxer;
> +extern AVOutputFormat ff_segafilm_muxer;
>  extern AVOutputFormat ff_segment_muxer;
>  extern AVOutputFormat ff_stream_segment_muxer;
>  extern AVInputFormat  ff_shorten_demuxer;
> diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c
> new file mode 100644
> index 00..cc0e21d4d3
> --- /dev/null
> +++ b/libavformat/segafilmenc.c
> @@ -0,0 +1,380 @@
> +/*
> + * Cinepak Video Decoder
> + * Copyright (C) 2003 The FFmpeg project
> + *
> + * 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.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +/**
> + * @file
> + * Sega FILM (.cpk) file muxer
> + * @author Misty De Meo 
> + *
> + * @see For more information regarding the Sega FILM file format, visit:
> + *   http://wiki.multimedia.cx/index.php?title=Sega_FILM
> + */
> +
> +#include "libavutil/intreadwrite.h"
> +#include "avformat.h"
> +#include "internal.h"
> +#include "avio_internal.h"
> +
> +typedef struct FILMPacket {
> +int audio;
> +int keyframe;
> +int32_t pts;
> +int32_t duration;
> +int32_t size;
> +int32_t index;
> +struct FILMPacket *next;
> +} FILMPacket;
> +
> +typedef struct FILMOutputContext {
> +const AVClass *class;
> +int audio_index;
> +int video_index;
> +int64_t stab_pos;
> +FILMPacket *start;
> +FILMPacket *last;
> +int64_t packet_count;
> +} FILMOutputContext;
> +
> +static int film_write_packet_to_header(AVFormatContext *format_context,
> FILMPacket *pkt)
> +{
> +AVIOContext *pb = format_context->pb;
> +// The bits in these two 32-bit integers contain info about the
> contents of this sample
> +int32_t info1 = 0;
> +int32_t info2 = 0;
> +
> +if (pkt->audio) {
> +// Always the same, carries no more information than "this is
> audio"
> +info1 = 0x;
> +info2 = 1;
> +} else {
> +info1 = pkt->pts;
> +info2 = pkt->duration;
> +// The top bit being set indicates a key frame
> +if (pkt->keyframe)
> +info1 |= (1 << 31);
> +}
> +
> +// Write the 16-byte sample info packet to the STAB chunk in the header
> +avio_wb32(pb, pkt->index);
> +avio_wb32(pb, pkt->size);
> +avio_wb32(pb, info1);
> +avio_wb32(pb, info2);
> +
> +return 0;
> +}
> +
> +static int film_write_packet(AVFormatContext *format_context, AVPacket
> *pkt)
> +{
> +FILMPacket *metadata = av_mallocz(sizeof(FILMPacket));
> +if (!metadata)
> +return AVERROR(ENOMEM);

Do not mix initializations and calling other functions. Here and in
others places.

> +AVIOContext *pb =