On Thu, Sep 12, 2013 at 4:59 AM, Diego Biurrun <[email protected]> wrote:
> 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? > I've shot off an email to them. > > --- > > configure | 4 +- > > libavformat/avisynth.c | 793 > ++++++++++++++++++++++++++++++++++++++----------- > > 2 files changed, 630 insertions(+), 167 deletions(-) > > What changed in this patch iteration? > I added the check to prevent 2.5.8 from being used, and re-added the return ret's into the read_packet function because not having them there caused scripts to not exit at the end and force the use of Ctrl+C to kill the process. > > --- 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? > The support of those four colorspaces is conditional on whether AviSynth or AvxSynth is being used, and the divide between the two is where the OS comes in. If Windows is not defined (and therefore AvxSynth is being used), those get forcibly disabled. > > +#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. > Aren't those necessary so that the entire {} block is defined as the meaning of LOAD_AVS_FUNC(name, continue_on_fail)? > > +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. > As mentioned above and noted in the comments, removing the return ret; lines and replacing them with a single return 0; at the end of the function cause video-only and audio-only scripts to not exit at the end of being read, forcing the user to have to Ctrl+C. It only happens when the script serves one and not the other - if both video and audio are present, the behavior doesn't occur and the process exits properly. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
