| Message ID | 20160803223324.5844-1-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi! 2016-08-04 0:33 GMT+02:00 James Almer <jamrial@gmail.com>: > This removes unnecessary header dependencies. Why is this an advantage? > 5 files changed, 25 insertions(+), 20 deletions(-) This doesn't imply a simplification... Carl Eugen
On 8/3/2016 7:59 PM, Carl Eugen Hoyos wrote: > Hi! > > 2016-08-04 0:33 GMT+02:00 James Almer <jamrial@gmail.com>: >> This removes unnecessary header dependencies. > > Why is this an advantage? h263, vc1 and mpeg decoder were pointlessly pulling the entire set of h264 headers because of it. > >> 5 files changed, 25 insertions(+), 20 deletions(-) > > This doesn't imply a simplification... The patch is not about simplifying the source files. And the extra lines are simply wrappers for the header inclusion. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2016-08-04 1:35 GMT+02:00 James Almer <jamrial@gmail.com>: > On 8/3/2016 7:59 PM, Carl Eugen Hoyos wrote: >> Hi! >> >> 2016-08-04 0:33 GMT+02:00 James Almer <jamrial@gmail.com>: >>> This removes unnecessary header dependencies. >> >> Why is this an advantage? > > h263, vc1 and mpeg decoder were pointlessly pulling the entire set > of h264 headers because of it. So the advantage is that compilation gets measurably faster with your patch? Preprocessor checks instead of conditions have the disadvantage that less syntax checking is done in some cases. Thank you, Carl Eugen
On 8/4/2016 4:23 AM, Carl Eugen Hoyos wrote: > 2016-08-04 1:35 GMT+02:00 James Almer <jamrial@gmail.com>: >> On 8/3/2016 7:59 PM, Carl Eugen Hoyos wrote: >>> Hi! >>> >>> 2016-08-04 0:33 GMT+02:00 James Almer <jamrial@gmail.com>: >>>> This removes unnecessary header dependencies. >>> >>> Why is this an advantage? >> >> h263, vc1 and mpeg decoder were pointlessly pulling the entire set >> of h264 headers because of it. > > So the advantage is that compilation gets measurably faster > with your patch? No, our headers are not the Boost package. i just want to reduce dependencies so they don't get recompiled every time you modify any of those 10+ unrelated headers or their dependencies. > > Preprocessor checks instead of conditions have the disadvantage > that less syntax checking is done in some cases. > > Thank you, Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Hi! 2016-08-04 16:01 GMT+02:00 James Almer <jamrial@gmail.com>: >> So the advantage is that compilation gets measurably faster >> with your patch? > > No, our headers are not the Boost package. i just want to reduce > dependencies so they don't get recompiled every time you modify > any of those 10+ unrelated headers or their dependencies. If you believe this helps, I can't object. Carl Eugen
On Fri, Aug 05, 2016 at 11:24:35PM +0200, Carl Eugen Hoyos wrote: > Hi! > > 2016-08-04 16:01 GMT+02:00 James Almer <jamrial@gmail.com>: > >> So the advantage is that compilation gets measurably faster > >> with your patch? > > > > No, our headers are not the Boost package. i just want to reduce > > dependencies so they don't get recompiled every time you modify > > any of those 10+ unrelated headers or their dependencies. > > If you believe this helps, I can't object. whats the status of this ? is this droped ? does it need a review ? iam asking due to pachwork showing this as "NEW" without being delegated to anyone. Updating statuses in patchwork is important to keep patchwork effective as a way to keep track of patches ... (it cannot know if a patch was dropped or is still waiting on somethig) thx [...]
On Sat, Sep 03, 2016 at 11:22:43AM +0200, Michael Niedermayer wrote: > On Fri, Aug 05, 2016 at 11:24:35PM +0200, Carl Eugen Hoyos wrote: > > Hi! > > > > 2016-08-04 16:01 GMT+02:00 James Almer <jamrial@gmail.com>: > > >> So the advantage is that compilation gets measurably faster > > >> with your patch? > > > > > > No, our headers are not the Boost package. i just want to reduce > > > dependencies so they don't get recompiled every time you modify > > > any of those 10+ unrelated headers or their dependencies. > > > > If you believe this helps, I can't object. > > whats the status of this ? > > is this droped ? > does it need a review ? > > iam asking due to pachwork showing this as "NEW" without being > delegated to anyone. Updating statuses in patchwork is important to > keep patchwork effective as a way to keep track of patches ... > (it cannot know if a patch was dropped or is still waiting on somethig) marked the patch as delegated to jamrial (i think thats correct as carl seems to have withdrawn his objection) [...]
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c index d0da1d3..953bbc5 100644 --- a/libavcodec/h263dec.c +++ b/libavcodec/h263dec.c @@ -41,7 +41,9 @@ #include "mpegvideo.h" #include "msmpeg4.h" #include "qpeldsp.h" +#if FF_API_CAP_VDPAU && CONFIG_MPEG4_VDPAU_DECODER #include "vdpau_compat.h" +#endif #include "thread.h" #include "wmv2.h" @@ -603,8 +605,8 @@ retry: if (!s->divx_packed && !avctx->hwaccel) ff_thread_finish_setup(avctx); -#if FF_API_CAP_VDPAU - if (CONFIG_MPEG4_VDPAU_DECODER && (s->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU)) { +#if FF_API_CAP_VDPAU && CONFIG_MPEG4_VDPAU_DECODER + if (s->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) { ff_vdpau_mpeg4_decode_picture(avctx->priv_data, s->gb.buffer, s->gb.buffer_end - s->gb.buffer); goto frame_end; } diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c index f634d2a..a721b8d 100644 --- a/libavcodec/h264_picture.c +++ b/libavcodec/h264_picture.c @@ -41,7 +41,9 @@ #include "mpegutils.h" #include "rectangle.h" #include "thread.h" +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER #include "vdpau_compat.h" +#endif void ff_h264_unref_picture(H264Context *h, H264Picture *pic) { @@ -156,9 +158,8 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup) int err = 0; h->mb_y = 0; -#if FF_API_CAP_VDPAU - if (CONFIG_H264_VDPAU_DECODER && - h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER + if (h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) ff_vdpau_h264_set_reference_frames(h); #endif @@ -179,9 +180,8 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup) "hardware accelerator failed to decode picture\n"); } -#if FF_API_CAP_VDPAU - if (CONFIG_H264_VDPAU_DECODER && - h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER + if (h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) ff_vdpau_h264_picture_complete(h); #endif diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 323639d..35df360 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -50,10 +50,13 @@ #include "mathops.h" #include "me_cmp.h" #include "mpegutils.h" +#include "mpegvideo.h" #include "profiles.h" #include "rectangle.h" #include "thread.h" +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER #include "vdpau_compat.h" +#endif static int h264_decode_end(AVCodecContext *avctx); @@ -844,9 +847,8 @@ again: if (h->avctx->hwaccel && (ret = h->avctx->hwaccel->start_frame(h->avctx, buf, buf_size)) < 0) goto end; -#if FF_API_CAP_VDPAU - if (CONFIG_H264_VDPAU_DECODER && - h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER + if (h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) ff_vdpau_h264_picture_start(h); #endif } @@ -858,9 +860,8 @@ again: nal->raw_size); if (ret < 0) goto end; -#if FF_API_CAP_VDPAU - } else if (CONFIG_H264_VDPAU_DECODER && - h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) { +#if FF_API_CAP_VDPAU && CONFIG_H264_VDPAU_DECODER + } else if (h->avctx->codec->capabilities & AV_CODEC_CAP_HWACCEL_VDPAU) { ff_vdpau_add_data_chunk(h->cur_pic_ptr->f->data[0], start_code, sizeof(start_code)); diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index 204a578..9f3daa3 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -47,7 +47,9 @@ #include "profiles.h" #include "thread.h" #include "version.h" +#if FF_API_VDPAU && (CONFIG_MPEG_VDPAU_DECODER || CONFIG_MPEG1_VDPAU_DECODER) #include "vdpau_compat.h" +#endif #include "xvmc_internal.h" typedef struct Mpeg1Context { @@ -2433,9 +2435,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture, s2->er.error_count += s2->thread_context[i]->er.error_count; } -#if FF_API_VDPAU - if ((CONFIG_MPEG_VDPAU_DECODER || CONFIG_MPEG1_VDPAU_DECODER) - && uses_vdpau(avctx)) +#if FF_API_VDPAU && (CONFIG_MPEG_VDPAU_DECODER || CONFIG_MPEG1_VDPAU_DECODER) + if (uses_vdpau(avctx)) ff_vdpau_mpeg_picture_complete(s2, buf, buf_size, s->slice_count); #endif diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c index 4f78aa8..dfe476a 100644 --- a/libavcodec/vc1dec.c +++ b/libavcodec/vc1dec.c @@ -37,7 +37,9 @@ #include "profiles.h" #include "vc1.h" #include "vc1data.h" +#if FF_API_CAP_VDPAU && CONFIG_VC1_VDPAU_DECODER #include "vdpau_compat.h" +#endif #include "libavutil/avassert.h" @@ -909,9 +911,8 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data, s->me.qpel_put = s->qdsp.put_qpel_pixels_tab; s->me.qpel_avg = s->qdsp.avg_qpel_pixels_tab; -#if FF_API_CAP_VDPAU - if ((CONFIG_VC1_VDPAU_DECODER) - &&s->avctx->codec->capabilities&AV_CODEC_CAP_HWACCEL_VDPAU) { +#if FF_API_CAP_VDPAU && CONFIG_VC1_VDPAU_DECODER + if (s->avctx->codec->capabilities&AV_CODEC_CAP_HWACCEL_VDPAU) { if (v->field_mode && buf_start_second_field) { ff_vdpau_vc1_decode_picture(s, buf_start, buf_start_second_field - buf_start); ff_vdpau_vc1_decode_picture(s, buf_start_second_field, (buf + buf_size) - buf_start_second_field);
This removes unnecessary header dependencies. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/h263dec.c | 6 ++++-- libavcodec/h264_picture.c | 12 ++++++------ libavcodec/h264dec.c | 13 +++++++------ libavcodec/mpeg12dec.c | 7 ++++--- libavcodec/vc1dec.c | 7 ++++--- 5 files changed, 25 insertions(+), 20 deletions(-)