On Wed, Sep 11, 2013 at 10:14:07PM -0400, Stephen Hutchinson wrote:
> From: d s <[email protected]>
I repeat: Can we please have a name to go along with this?
> ---
> configure | 4 +-
> libavformat/avisynth.c | 793
> ++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 630 insertions(+), 167 deletions(-)
What changed in this patch iteration?
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -19,211 +19,672 @@
> +
> +// AvxSynth doesn't have these colorspaces, so disable them
> +#ifndef _WIN32
> +#define avs_is_yv24(vi) 0
> +#define avs_is_yv16(vi) 0
> +#define avs_is_yv411(vi) 0
> +#define avs_is_y8(vi) 0
> +#endif
The comment contradicts the ifdef condition. How is the support of
colorspaces related to the operating system?
> +#define LOAD_AVS_FUNC(name, continue_on_fail) \
> + { \
> + avs_library->name = \
> + (void *)GetProcAddress(avs_library->library, #name); \
> + if (!continue_on_fail && !avs_library->name) \
> + goto fail; \
> + }
> + LOAD_AVS_FUNC(avs_bit_blt, 0);
> + LOAD_AVS_FUNC(avs_clip_get_error, 0);
The extra {} look unnecessary.
> +static void avisynth_next_stream(AVFormatContext *s, AVStream **st,
> + AVPacket *pkt, int *discard)
> +{
> + AviSynthContext *avs = s->priv_data;
> +
> + pkt->stream_index = avs->curr_stream++;
> + avs->curr_stream %= s->nb_streams;
> +
> + *st = s->streams[pkt->stream_index];
> + if ((*st)->discard == AVDISCARD_ALL)
> + *discard = 1;
> + else
> + *discard = 0;
> +
> + return;
> }
You pass st as **, but dereference it on each use. Why not pass it as
a simple pointer to begin with?
> +static int avisynth_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> + AviSynthContext *avs = s->priv_data;
> + AVStream *st;
> + int discard = 0;
> + int ret;
> +
> + if (avs->error)
> + return AVERROR_UNKNOWN;
> +
> + pkt->destruct = av_destruct_packet;
> +
> + // If either stream reaches EOF, try to read the other one before giving
> up.
> + avisynth_next_stream(s, &st, pkt, &discard);
> + if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
> + ret = avisynth_read_packet_video(s, pkt, discard);
> + if (ret == AVERROR_EOF && avs_has_audio(avs->vi)) {
> + avisynth_next_stream(s, &st, pkt, &discard);
> + return avisynth_read_packet_audio(s, pkt, discard);
> + }
> + return ret; // so that video-only scripts don't hang at end of read
> + } else {
> + ret = avisynth_read_packet_audio(s, pkt, discard);
> + if (ret == AVERROR_EOF && avs_has_video(avs->vi)) {
> + avisynth_next_stream(s, &st, pkt, &discard);
> + return avisynth_read_packet_video(s, pkt, discard);
> + }
> + return ret; // so that audio-only scripts don't hang at end of read
> + }
> +}
You can factor out the return.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
