On Wed, Sep 04, 2013 at 11:29:02AM -0400, Stephen Hutchinson wrote:
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -21,203 +21,660 @@
>
> #include "avformat.h"
> #include "internal.h"
> -#include "riff.h"
> -
> -#include <windows.h>
> -#include <vfw.h>
> +#include "libavcodec/internal.h"
> +
> +// Enable function pointer definitions for runtime loading.
> +#define AVSC_NO_DECLSPEC
> +
> +// Shut up error messages.
> +// avisynth_c.h contains inline functions that call these functions.
> +#undef malloc
> +#undef free
> +#undef printf
These comments appear outdated.
> +// Platform-specific directives for AviSynth vs AvxSynth.
> +#ifdef _WIN32
> + #include <windows.h>
> + #undef EXTERN_C
> + #include <avisynth/avisynth_c.h>
> + #include <avisynth/avisynth_c_25.h>
> + #define AVISYNTH_LIB "avisynth"
> +#else
> + #include <dlfcn.h>
> + #include <avxsynth/avxsynth_c.h>
> + #if defined (__APPLE__)
> + #define AVISYNTH_LIB "libavxsynth.dylib"
> + #else
> + #define AVISYNTH_LIB "libavxsynth.so"
> + #endif
> +
> + #define LoadLibrary(x) dlopen(x, RTLD_NOW | RTLD_GLOBAL)
> + #define GetProcAddress dlsym
> + #define FreeLibrary dlclose
> +#endif
What is this ifdeffery for?
> typedef struct {
> + void *library;
> +#define AVSC_DECLARE_FUNC(name) name##_func name
> + AVSC_DECLARE_FUNC(avs_bit_blt);
> + AVSC_DECLARE_FUNC(avs_clip_get_error);
> + AVSC_DECLARE_FUNC(avs_create_script_environment);
> + AVSC_DECLARE_FUNC(avs_delete_script_environment);
> + AVSC_DECLARE_FUNC(avs_get_audio);
> + AVSC_DECLARE_FUNC(avs_get_error);
> + AVSC_DECLARE_FUNC(avs_get_frame);
> + AVSC_DECLARE_FUNC(avs_get_version);
> + AVSC_DECLARE_FUNC(avs_get_video_info);
> + AVSC_DECLARE_FUNC(avs_invoke);
> + AVSC_DECLARE_FUNC(avs_release_clip);
> + AVSC_DECLARE_FUNC(avs_release_value);
> + AVSC_DECLARE_FUNC(avs_release_video_frame);
> + AVSC_DECLARE_FUNC(avs_take_clip);
> +#undef AVSC_DECLARE_FUNC
> +} AviSynthLibrary;
Don't anonymously declare structs, this can cause problems.
> +static const int avs_planes_packed[1] = {0};
> +static const int avs_planes_grey[1] = {AVS_PLANAR_Y};
> +static const int avs_planes_yuv[3] = {AVS_PLANAR_Y, AVS_PLANAR_U,
> AVS_PLANAR_V};
spaces inside {}
> +static av_cold int avisynth_load_library(void)
> {
> + avs_library = av_mallocz(sizeof(AviSynthLibrary));
> + if (!avs_library)
> + return AVERROR_UNKNOWN;
AVERROR(ENOMEM)
> + avs_library->library = LoadLibrary(AVISYNTH_LIB);
> + if (!avs_library->library)
> + goto init_fail;
> +
> +#define LOAD_AVS_FUNC(name, continue_on_fail) \
> +{ \
> + avs_library->name = (void*)GetProcAddress(avs_library->library, #name); \
> + if(!continue_on_fail && !avs_library->name) \
if (
> +// Note that avisynth_context_create and avisynth_context_destroy
> +// do not allocate or free the actual context! That is taken care of
> +// by libavformat.
> +static av_cold int avisynth_context_create(AVFormatContext *s)
> +{
> + AviSynthContext *avs = (AviSynthContext *)s->priv_data;
Looks like a pointless void* cast.
> +// Create AVStream from audio and video data.
> +static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
> +{
> + AviSynthContext *avs = s->priv_data;
> + int planar = 0; // 0: packed, 1: YUV, 2: Y8
> +
> + st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
> + st->codec->codec_id = AV_CODEC_ID_RAWVIDEO;
> + st->codec->width = avs->vi->width;
> + st->codec->height = avs->vi->height;
> +
> + st->time_base = (AVRational) {avs->vi->fps_denominator,
> avs->vi->fps_numerator};
> + st->avg_frame_rate = (AVRational) {avs->vi->fps_numerator,
> avs->vi->fps_denominator};
> + st->start_time = 0;
> + st->duration = avs->vi->num_frames;
> + st->nb_frames = avs->vi->num_frames;
Align some of these =.
more below
> +static int avisynth_open_file(AVFormatContext *s)
> {
> + AviSynthContext *avs = (AviSynthContext *)s->priv_data;
> + AVS_Value arg, val;
> + int ret;
> +#ifdef _WIN32
> + char filename_ansi[MAX_PATH * 4];
> + wchar_t filename_wc[MAX_PATH * 4];
> +#endif
> +
> + if (ret = avisynth_context_create(s))
> + return ret;
> +
> +#ifdef _WIN32
> + // Convert UTF-8 to ANSI code page
> + MultiByteToWideChar(CP_UTF8, 0, s->filename, -1, filename_wc, MAX_PATH *
> 4);
> + WideCharToMultiByte(CP_THREAD_ACP, 0, filename_wc, -1, filename_ansi,
> MAX_PATH * 4, NULL, NULL);
> + arg = avs_new_value_string(filename_ansi);
> +#else
> + arg = avs_new_value_string(s->filename);
> +#endif
What is this about?
> +static void avisynth_next_stream(AVFormatContext *s, AVStream **st,
> + AVPacket *pkt, int *discard)
Indentation is off, more below.
> + // Define the bpp values for the new AviSynth 2.6 colorspaces
> + if (avs_is_yv24(avs->vi)) {
> + bits = 24;
> + } else if (avs_is_yv16(avs->vi)) {
> + bits = 16;
> + } else if (avs_is_yv411(avs->vi)) {
> + bits = 12;
> + } else if (avs_is_y8(avs->vi)) {
> + bits = 8;
> + } else {
> + bits = avs_bits_per_pixel(avs->vi);
> + }
Drop the {}
> +#ifdef _WIN32
> + if (avs_library->avs_get_version(avs->clip) == 3) {
> + rowsize = avs_get_row_size_p_25(frame, plane);
> + planeheight = avs_get_height_p_25(frame, plane);
> + } else {
> + rowsize = avs_get_row_size_p(frame, plane);
> + planeheight = avs_get_height_p(frame, plane);
> + }
> +#else
> + rowsize = avs_get_row_size_p(frame, plane);
> + planeheight = avs_get_height_p(frame, plane);
> +#endif
The else and the #else case look identical.
> + // 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;
> + } 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;
> + }
> }
Factor out the return.
Try to keep lines below 80 characters where easily possible.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
