On Sun, Jun 12, 2016 at 02:43:10AM +0100, Josh de Kock wrote: > --- > configure | 4 + > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/libopenmpt.c | 185 > +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 191 insertions(+) > create mode 100644 libavformat/libopenmpt.c > > diff --git a/configure b/configure > index 7c463a5..a74aaab 100755 > --- a/configure > +++ b/configure > @@ -248,6 +248,7 @@ External library support: > --enable-libopencv enable video filtering via libopencv [no] > --enable-libopenh264 enable H.264 encoding via OpenH264 [no] > --enable-libopenjpeg enable JPEG 2000 de/encoding via OpenJPEG [no]
> + --enable-libopenmpt enable decoding tracked mod files via libopenmpt
> [no]
libopenmpt is supposed to replace libmodplug support all kind of trackers,
not only MOD.
> --enable-libopus enable Opus de/encoding via libopus [no]
> --enable-libpulse enable Pulseaudio input via libpulse [no]
> --enable-librubberband enable rubberband needed for rubberband filter
> [no]
> @@ -1495,6 +1496,7 @@ EXTERNAL_LIBRARY_LIST="
> libopencv
> libopenh264
> libopenjpeg
> + libopenmpt
> libopus
> libpulse
> librtmp
> @@ -2741,6 +2743,7 @@ libopencore_amrwb_decoder_deps="libopencore_amrwb"
> libopenh264_encoder_deps="libopenh264"
> libopenjpeg_decoder_deps="libopenjpeg"
> libopenjpeg_encoder_deps="libopenjpeg"
> +libopenmpt_demuxer_deps="libopenmpt"
> libopus_decoder_deps="libopus"
> libopus_encoder_deps="libopus"
> libopus_encoder_select="audio_frame_queue"
> @@ -5633,6 +5636,7 @@ enabled libopenjpeg && { check_lib
> openjpeg-2.1/openjpeg.h opj_version -lo
> check_lib openjpeg-1.5/openjpeg.h opj_version
> -lopenjpeg -DOPJ_STATIC ||
> check_lib openjpeg.h opj_version -lopenjpeg
> -DOPJ_STATIC ||
> die "ERROR: libopenjpeg not found"; }
> +enabled libopenmpt && require libopenmpt libopenmpt/libopenmpt.h
> openmpt_module_create -lopenmpt -lstdc++
openmpt is distributed with pkg-config so please use require_pkg_config.
And then you shouldn't need -lstdc++ if the .pc is properly done.
[...]
> +/**
> +* @file
> +* libopenmpt demuxer
> +*/
not really useful
> +
> +#include <libopenmpt/libopenmpt.h>
> +#include "libavutil/avstring.h"
> +#include "libavutil/eval.h"
doesn't look necessary (maybe check the other includes)
> +#include "libavutil/opt.h"
> +#include "avformat.h"
> +#include "internal.h"
> +
> +typedef struct OpenMPTContext {
> + const AVClass *class;
> + openmpt_module *module;
> +
> + int channels;
> + double length;
better call this duration for consistency
> + /* options */
> + int sample_rate;
> +} OpenMPTContext;
> +
> +#define OFFSET(x) offsetof(OpenMPTContext, x)
> +#define A AV_OPT_FLAG_AUDIO_PARAM
> +#define D AV_OPT_FLAG_DECODING_PARAM
> +static const AVOption options[] = {
> + {"sample_rate", "set sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT,
> {.i64 = 44100}, 1000, 999999, A|D},
INT_MAX instead of 999999?
> + {NULL}
> +};
> +
> +static int probe_openmpt(AVProbeData *p)
> +{
> + if (p->buf_size < 1084)
> + return 0;
> +
> + if (p->buf[1080] == 'M' && p->buf[1081] == '.' &&
> + p->buf[1082] == 'K' && p->buf[1083] == '.')
> + return AVPROBE_SCORE_MAX;
> +
So this is going to probe only one kind of tracker? It would be nice to
detect all the tracker files the library supports. With modplug we rely on
extension (bad); does openmpt support something better (something better
than trying to decode, which will be slow and slow down every other
probing)?
> + return 0;
> +}
> +
> +static void openmpt_logfunc(const char *message, void *userdata)
> +{
> + av_log((AVFormatContext*)userdata, AV_LOG_INFO, "%s\n", message);
The cast is not necessary. No loglevel to differenciate error from
warnings from info? That's a shame.
> +}
> +
> +#define add_meta(s, name, value) \
> + if (value && value[0]) \
> + av_dict_set(&s->metadata, name, value, 0); \
> +
nit: scope this in a do { ... } while (0) form
> +static int read_header_openmpt(AVFormatContext *s)
> +{
> + AVStream *st;
> + OpenMPTContext *openmpt = s->priv_data;
> + int64_t size = avio_size(s->pb);
> + char *buf = av_malloc(size);
> +
> + if (!buf)
> + return AVERROR(ENOMEM);
> + size = avio_read(s->pb, buf, size);
> +
> + if (!(openmpt->module = openmpt_module_create_from_memory(buf, size,
> openmpt_logfunc, s, NULL))) {
> + av_free(buf);
> + return AVERROR_INVALIDDATA;
> + }
> + av_free(buf);
openmpt->module = openmpt_module_create_from_memory(buf, size,
openmpt_logfunc, s, NULL);
av_freep(&buf);
if (!openmpt->module)
return AVERROR_EXTERNAL;
(or maybe AVERROR(ENOMEM)?)
> + openmpt->channels = openmpt_module_get_num_channels(openmpt->module);
Only a number of channels; no channel layout provided?
> + openmpt->length = openmpt_module_get_duration_seconds(openmpt->module);
> +
> + add_meta(s, "artist", openmpt_module_get_metadata(openmpt->module,
> "artist"));
> + add_meta(s, "title", openmpt_module_get_metadata(openmpt->module,
> "title"));
> + add_meta(s, "encoder", openmpt_module_get_metadata(openmpt->module,
> "tracker"));
> + add_meta(s, "comment", openmpt_module_get_metadata(openmpt->module,
> "message"));
add_meta() is going to call the same function 3 times (you could have a
local const char *value in the local do { ... } while (0) suggested.)
> +
> + st = avformat_new_stream(s, NULL);
> + if (!st)
> + return AVERROR(ENOMEM);
> + avpriv_set_pts_info(st, 64, 1, 1000);
> + if (st->duration > 0)
> + st->duration = openmpt->length;
Is this really in double?
> +
> + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> + st->codecpar->codec_id = AV_NE(AV_CODEC_ID_PCM_S16BE,
> AV_CODEC_ID_PCM_S16LE);
> + st->codecpar->channels = openmpt->channels;
> + st->codecpar->sample_rate = openmpt->sample_rate;
> +
> + return 0;
> +}
> +
> +#define AUDIO_PKT_SIZE 2048
> +
> +static int read_packet_openmpt(AVFormatContext *s, AVPacket *pkt)
> +{
> + OpenMPTContext *openmpt = s->priv_data;
> + int n_samples = AUDIO_PKT_SIZE / (openmpt->channels ?
> openmpt->channels*2 : 2);
> + int ret;
> +
> + if ((ret = av_new_packet(pkt, AUDIO_PKT_SIZE)) < 0)
> + return ret;
> +
> + switch (openmpt->channels) {
> + case 1: {
> + ret = openmpt_module_read_mono(openmpt->module, openmpt->sample_rate,
> + n_samples, (short *)pkt->data);
> + break;
> + }
you don't need to scope each case as you're not declaring ret (or any
other variable) inside them.
> + case 2: {
> + ret = openmpt_module_read_interleaved_stereo(openmpt->module,
> openmpt->sample_rate,
> + n_samples, (short
> *)pkt->data);
> + break;
> + }
> + case 4: {
> + ret = openmpt_module_read_interleaved_quad(openmpt->module,
> openmpt->sample_rate,
> + n_samples, (short
> *)pkt->data);
> + break;
> + }
> + default:
> + av_log(s, AV_LOG_ERROR, "Invalid amount of channels: %d",
> openmpt->channels);
I'm not a native speaker so correct me if I'm wrong but using "amount"
sounds weird here; you probably want "number".
I'd also use "Unsupported" instead of "Invalid" (and adjust the code
message).
> + return AVERROR(EINVAL);
> + }
> +
> + if (ret < 1 || (openmpt->length <
> openmpt_module_get_position_seconds(openmpt->module)))
> + return AVERROR_EOF;
> +
> + pkt->size = AUDIO_PKT_SIZE;
aren't you supposed to use pkt->size = ret?
> +
> + return 0;
> +}
> +
> +static int read_close_openmpt(AVFormatContext *s)
> +{
> + OpenMPTContext *openmpt = s->priv_data;
> + openmpt_module_destroy(openmpt->module);
> + return 0;
> +}
> +
> +static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts,
> int flags)
> +{
> + OpenMPTContext *openmpt = s->priv_data;
> + openmpt_module_set_position_seconds(openmpt->module,
> (double)ts/AV_TIME_BASE);
> + return 0;
> +}
> +
> +static const AVClass class_openmpt = {
> + .class_name = "libopenmpt",
> + .item_name = av_default_item_name,
> + .option = options,
> + .version = LIBAVUTIL_VERSION_INT,
> +};
> +
> +AVInputFormat ff_libopenmpt_demuxer = {
> + .name = "libopenmpt",
> + .long_name = NULL_IF_CONFIG_SMALL("Tracker MOD format
> (libopenmpt)"),
> + .priv_data_size = sizeof(OpenMPTContext),
> + .read_probe = probe_openmpt,
> + .read_header = read_header_openmpt,
> + .read_packet = read_packet_openmpt,
> + .read_close = read_close_openmpt,
> + .read_seek = read_seek_openmpt,
> + .priv_class = &class_openmpt,
> + .extensions = "mod",
> +};
Note: please add "TODO: bump lavf minor" in the commit description so the
commiter doesn't forget. You should also reference the trac ticket.
And you can update the Changelog as well.
--
Clément B.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
