diff mbox

[FFmpeg-devel,1/7] avcodec: do not use AVFrame accessor

Message ID 20170422090428.6459-1-mfcc64@gmail.com
State Accepted
Headers show

Commit Message

Muhammad Faiz April 22, 2017, 9:04 a.m. UTC
Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
---
 libavcodec/ac3dec.c     |  2 +-
 libavcodec/cpia.c       |  8 +++----
 libavcodec/cuvid.c      |  6 ++---
 libavcodec/decode.c     | 62 ++++++++++++++++++++++++-------------------------
 libavcodec/encode.c     |  2 +-
 libavcodec/exr.c        |  2 +-
 libavcodec/gifdec.c     |  2 +-
 libavcodec/mjpegdec.c   |  2 +-
 libavcodec/pngdec.c     |  2 +-
 libavcodec/proresdec2.c |  2 +-
 libavcodec/rawdec.c     |  4 ++--
 libavcodec/tiff.c       |  8 +++----
 libavcodec/webp.c       |  2 +-
 13 files changed, 51 insertions(+), 53 deletions(-)

Comments

wm4 April 22, 2017, 9:27 a.m. UTC | #1
On Sat, 22 Apr 2017 16:04:22 +0700
Muhammad Faiz <mfcc64@gmail.com> wrote:

> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> ---

+1 to patches 1-7. As long as the accessors only trivially access
public fields, it's better (and less ugly) not to use accessors at all.
Aaron Levinson April 22, 2017, 6:55 p.m. UTC | #2
On 4/22/2017 2:27 AM, wm4 wrote:
> On Sat, 22 Apr 2017 16:04:22 +0700
> Muhammad Faiz <mfcc64@gmail.com> wrote:
>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>
> +1 to patches 1-7. As long as the accessors only trivially access
> public fields, it's better (and less ugly) not to use accessors at all.

Then why have the accessors at all if the fields are public?  As far as 
I can tell, the benefit to using these accessors is that, if certain 
properties become internal in the future, or variable names change, or 
the structures become opaque, etc, the code will continue to work.

Aaron Levinson
Nicolas George April 22, 2017, 6:57 p.m. UTC | #3
Le tridi 3 floréal, an CCXXV, Aaron Levinson a écrit :
> Then why have the accessors at all if the fields are public?

To ensure ABI compatibility with the fork, which has been dropped.

Regards,
Hendrik Leppkes April 22, 2017, 9:54 p.m. UTC | #4
On Sat, Apr 22, 2017 at 8:57 PM, Nicolas George <george@nsup.org> wrote:
> Le tridi 3 floréal, an CCXXV, Aaron Levinson a écrit :
>> Then why have the accessors at all if the fields are public?
>
> To ensure ABI compatibility with the fork, which has been dropped.
>

To elaborate on that, the accessors existed primarily for fields which
only exist in FFmpeg, and not in Libav, so that when Libav added a
field, we could add it in the same position as them, and preserve ABI
compatibility to Libav (ie. having all fields in the struct in the
same place).
Due to this practice, any fields Libav didn't have had to be at the
end of the struct, and they would move everytime a new field was added
on Libavs side (which would end up before the FFmpeg-only fields) -
hence accessors to work around the constant ABI changes to these
fields.

But we have decided a while ago that we do no longer want to pretent
to maintain ABI compat (it was never fully functional and basically
untested), and as such there is no need to have them anymore (it was a
bit clumsy to use either  way, as only a minority of fields had them,
so it was easy to use it wrong).

On top of these changes, the accessors should also be deprecated and
then eventually removed (after the typical deprecation period), and
all public fields in the structs accessed directly again.

- Hendrik
Muhammad Faiz April 23, 2017, 8:16 a.m. UTC | #5
On Sat, Apr 22, 2017 at 4:27 PM, wm4 <nfxjfg@googlemail.com> wrote:
> On Sat, 22 Apr 2017 16:04:22 +0700
> Muhammad Faiz <mfcc64@gmail.com> wrote:
>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>
> +1 to patches 1-7. As long as the accessors only trivially access
> public fields, it's better (and less ugly) not to use accessors at all.

add some missing changes and applied

thank's
diff mbox

Patch

diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index 4a0d8bb..7e2cbce 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -1622,7 +1622,7 @@  static int ac3_decode_frame(AVCodecContext * avctx, void *data,
         }
     }
 
-    av_frame_set_decode_error_flags(frame, err ? FF_DECODE_ERROR_INVALID_BITSTREAM : 0);
+    frame->decode_error_flags = err ? FF_DECODE_ERROR_INVALID_BITSTREAM : 0;
 
     /* keep last block for error concealment in next frame */
     for (ch = 0; ch < s->out_channels; ch++)
diff --git a/libavcodec/cpia.c b/libavcodec/cpia.c
index 07cdd50..58833b2 100644
--- a/libavcodec/cpia.c
+++ b/libavcodec/cpia.c
@@ -113,12 +113,12 @@  static int cpia_decode_frame(AVCodecContext *avctx,
         src += 2;
 
         if (src_size < linelength) {
-            av_frame_set_decode_error_flags(frame, FF_DECODE_ERROR_INVALID_BITSTREAM);
+            frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
             av_log(avctx, AV_LOG_WARNING, "Frame ended unexpectedly!\n");
             break;
         }
         if (src[linelength - 1] != EOL) {
-            av_frame_set_decode_error_flags(frame, FF_DECODE_ERROR_INVALID_BITSTREAM);
+            frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
             av_log(avctx, AV_LOG_WARNING, "Wrong line length %d or line not terminated properly (found 0x%02x)!\n", linelength, src[linelength - 1]);
             break;
         }
@@ -139,7 +139,7 @@  static int cpia_decode_frame(AVCodecContext *avctx,
              */
             for (j = 0; j < linelength - 1; j++) {
                 if (y > y_end) {
-                    av_frame_set_decode_error_flags(frame, FF_DECODE_ERROR_INVALID_BITSTREAM);
+                    frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
                     av_log(avctx, AV_LOG_WARNING, "Decoded data exceeded linesize!\n");
                     break;
                 }
@@ -159,7 +159,7 @@  static int cpia_decode_frame(AVCodecContext *avctx,
              */
             for (j = 0; j < linelength - 4; ) {
                 if (y + 1 > y_end || u > u_end || v > v_end) {
-                    av_frame_set_decode_error_flags(frame, FF_DECODE_ERROR_INVALID_BITSTREAM);
+                    frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
                     av_log(avctx, AV_LOG_WARNING, "Decoded data exceeded linesize!\n");
                     break;
                 }
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 916d7e9..71f802c 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -582,9 +582,9 @@  FF_DISABLE_DEPRECATION_WARNINGS
         frame->pkt_pts = frame->pts;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
-        av_frame_set_pkt_pos(frame, -1);
-        av_frame_set_pkt_duration(frame, 0);
-        av_frame_set_pkt_size(frame, -1);
+        frame->pkt_pos = -1;
+        frame->pkt_duration = 0;
+        frame->pkt_size = -1;
 
         frame->interlaced_frame = !parsed_frame.is_deinterlacing && !parsed_frame.dispinfo.progressive_frame;
 
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index df9af68..0aaa6c5 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -136,7 +136,7 @@  static int unrefcount_frame(AVCodecInternal *avci, AVFrame *frame)
     memcpy(frame->data,     avci->to_free->data,     sizeof(frame->data));
     memcpy(frame->linesize, avci->to_free->linesize, sizeof(frame->linesize));
     if (avci->to_free->extended_data != avci->to_free->data) {
-        int planes = av_frame_get_channels(avci->to_free);
+        int planes = avci->to_free->channels;
         int size   = planes * sizeof(*frame->extended_data);
 
         if (!size) {
@@ -159,7 +159,7 @@  static int unrefcount_frame(AVCodecInternal *avci, AVFrame *frame)
     frame->height         = avci->to_free->height;
     frame->channel_layout = avci->to_free->channel_layout;
     frame->nb_samples     = avci->to_free->nb_samples;
-    av_frame_set_channels(frame, av_frame_get_channels(avci->to_free));
+    frame->channels       = avci->to_free->channels;
 
     return 0;
 }
@@ -333,9 +333,9 @@  int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
             return AVERROR_EOF;
         ret = avctx->codec->receive_frame(avctx, frame);
         if (ret >= 0) {
-            if (av_frame_get_best_effort_timestamp(frame) == AV_NOPTS_VALUE) {
-                av_frame_set_best_effort_timestamp(frame,
-                    guess_correct_pts(avctx, frame->pts, frame->pkt_dts));
+            if (frame->best_effort_timestamp == AV_NOPTS_VALUE) {
+                frame->best_effort_timestamp =
+                    guess_correct_pts(avctx, frame->pts, frame->pkt_dts);
             }
         }
         return ret;
@@ -423,7 +423,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 picture->pkt_dts = avpkt->dts;
 
             if(!avctx->has_b_frames){
-                av_frame_set_pkt_pos(picture, avpkt->pos);
+                picture->pkt_pos = avpkt->pos;
             }
             //FIXME these should be under if(!avctx->has_b_frames)
             /* get_buffer is supposed to set frame parameters */
@@ -457,10 +457,9 @@  fail:
             }
 
             avctx->frame_number++;
-            av_frame_set_best_effort_timestamp(picture,
-                                               guess_correct_pts(avctx,
-                                                                 picture->pts,
-                                                                 picture->pkt_dts));
+            picture->best_effort_timestamp = guess_correct_pts(avctx,
+                                                               picture->pts,
+                                                               picture->pkt_dts);
         } else
             av_frame_unref(picture);
     } else
@@ -534,16 +533,15 @@  FF_ENABLE_DEPRECATION_WARNINGS
         }
         if (ret >= 0 && *got_frame_ptr) {
             avctx->frame_number++;
-            av_frame_set_best_effort_timestamp(frame,
-                                               guess_correct_pts(avctx,
-                                                                 frame->pts,
-                                                                 frame->pkt_dts));
+            frame->best_effort_timestamp = guess_correct_pts(avctx,
+                                                             frame->pts,
+                                                             frame->pkt_dts);
             if (frame->format == AV_SAMPLE_FMT_NONE)
                 frame->format = avctx->sample_fmt;
             if (!frame->channel_layout)
                 frame->channel_layout = avctx->channel_layout;
-            if (!av_frame_get_channels(frame))
-                av_frame_set_channels(frame, avctx->channels);
+            if (!frame->channels)
+                frame->channels = avctx->channels;
             if (!frame->sample_rate)
                 frame->sample_rate = avctx->sample_rate;
         }
@@ -588,8 +586,8 @@  FF_ENABLE_DEPRECATION_WARNINGS
 #endif
                     if(frame->pkt_dts!=AV_NOPTS_VALUE)
                         frame->pkt_dts += diff_ts;
-                    if (av_frame_get_pkt_duration(frame) >= diff_ts)
-                        av_frame_set_pkt_duration(frame, av_frame_get_pkt_duration(frame) - diff_ts);
+                    if (frame->pkt_duration >= diff_ts)
+                        frame->pkt_duration -= diff_ts;
                 } else {
                     av_log(avctx, AV_LOG_WARNING, "Could not update timestamps for skipped samples.\n");
                 }
@@ -609,7 +607,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
                     int64_t diff_ts = av_rescale_q(frame->nb_samples - discard_padding,
                                                    (AVRational){1, avctx->sample_rate},
                                                    avctx->pkt_timebase);
-                    av_frame_set_pkt_duration(frame, diff_ts);
+                    frame->pkt_duration = diff_ts;
                 } else {
                     av_log(avctx, AV_LOG_WARNING, "Could not update timestamps for discarded samples.\n");
                 }
@@ -1128,7 +1126,7 @@  static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
         break;
         }
     case AVMEDIA_TYPE_AUDIO: {
-        int ch     = av_frame_get_channels(frame); //av_get_channel_layout_nb_channels(frame->channel_layout);
+        int ch     = frame->channels; //av_get_channel_layout_nb_channels(frame->channel_layout);
         int planar = av_sample_fmt_is_planar(frame->format);
         int planes = planar ? ch : 1;
 
@@ -1283,7 +1281,7 @@  static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
     int size;
     const uint8_t *side_metadata;
 
-    AVDictionary **frame_md = avpriv_frame_get_metadatap(frame);
+    AVDictionary **frame_md = &frame->metadata;
 
     side_metadata = av_packet_get_side_data(avpkt,
                                             AV_PKT_DATA_STRINGS_METADATA, &size);
@@ -1314,9 +1312,9 @@  FF_DISABLE_DEPRECATION_WARNINGS
         frame->pkt_pts = pkt->pts;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
-        av_frame_set_pkt_pos     (frame, pkt->pos);
-        av_frame_set_pkt_duration(frame, pkt->duration);
-        av_frame_set_pkt_size    (frame, pkt->size);
+        frame->pkt_pos      = pkt->pos;
+        frame->pkt_duration = pkt->duration;
+        frame->pkt_size     = pkt->size;
 
         for (i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
             int size;
@@ -1345,9 +1343,9 @@  FF_DISABLE_DEPRECATION_WARNINGS
         frame->pkt_pts = AV_NOPTS_VALUE;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
-        av_frame_set_pkt_pos     (frame, -1);
-        av_frame_set_pkt_duration(frame, 0);
-        av_frame_set_pkt_size    (frame, -1);
+        frame->pkt_pos      = -1;
+        frame->pkt_duration = 0;
+        frame->pkt_size     = -1;
     }
     frame->reordered_opaque = avctx->reordered_opaque;
 
@@ -1355,10 +1353,10 @@  FF_ENABLE_DEPRECATION_WARNINGS
         frame->color_primaries = avctx->color_primaries;
     if (frame->color_trc == AVCOL_TRC_UNSPECIFIED)
         frame->color_trc = avctx->color_trc;
-    if (av_frame_get_colorspace(frame) == AVCOL_SPC_UNSPECIFIED)
-        av_frame_set_colorspace(frame, avctx->colorspace);
-    if (av_frame_get_color_range(frame) == AVCOL_RANGE_UNSPECIFIED)
-        av_frame_set_color_range(frame, avctx->color_range);
+    if (frame->colorspace == AVCOL_SPC_UNSPECIFIED)
+        frame->colorspace = avctx->colorspace;
+    if (frame->color_range == AVCOL_RANGE_UNSPECIFIED)
+        frame->color_range = avctx->color_range;
     if (frame->chroma_location == AVCHROMA_LOC_UNSPECIFIED)
         frame->chroma_location = avctx->chroma_sample_location;
 
@@ -1401,7 +1399,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 }
             }
         }
-        av_frame_set_channels(frame, avctx->channels);
+        frame->channels = avctx->channels;
         break;
     }
     return 0;
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 6e9d487..525ee1f 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -88,7 +88,7 @@  static int pad_last_frame(AVCodecContext *s, AVFrame **dst, const AVFrame *src)
 
     frame->format         = src->format;
     frame->channel_layout = src->channel_layout;
-    av_frame_set_channels(frame, av_frame_get_channels(src));
+    frame->channels       = src->channels;
     frame->nb_samples     = s->frame_size;
     ret = av_frame_get_buffer(frame, 32);
     if (ret < 0)
diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 7cf3620..7194640 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1630,7 +1630,7 @@  static int decode_header(EXRContext *s, AVFrame *frame)
         return AVERROR_INVALIDDATA;
     }
 
-    av_frame_set_metadata(frame, metadata);
+    frame->metadata = metadata;
 
     // aaand we are done
     bytestream2_skip(&s->gb, 1);
diff --git a/libavcodec/gifdec.c b/libavcodec/gifdec.c
index 48345db..2eeed4c 100644
--- a/libavcodec/gifdec.c
+++ b/libavcodec/gifdec.c
@@ -468,7 +468,7 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
     s->frame->pkt_dts = avpkt->dts;
-    av_frame_set_pkt_duration(s->frame, avpkt->duration);
+    s->frame->pkt_duration = avpkt->duration;
 
     if (avpkt->size >= 6) {
         s->keyframe = memcmp(avpkt->data, gif87a_sig, 6) == 0 ||
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 98cbd41..a86f6b2 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2499,7 +2499,7 @@  the_end:
         av_freep(&s->stereo3d);
     }
 
-    av_dict_copy(avpriv_frame_get_metadatap(data), s->exif_metadata, 0);
+    av_dict_copy(&((AVFrame *) data)->metadata, s->exif_metadata, 0);
     av_dict_free(&s->exif_metadata);
 
 the_end_no_picture:
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 1025519..aece1fc 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1171,7 +1171,7 @@  static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
             }
         }
 
-        metadatap = avpriv_frame_get_metadatap(p);
+        metadatap = &p->metadata;
         switch (tag) {
         case MKTAG('I', 'H', 'D', 'R'):
             if ((ret = decode_ihdr_chunk(avctx, s, length)) < 0)
diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index ff46bcf..73b161f 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -622,7 +622,7 @@  static int decode_picture(AVCodecContext *avctx)
         error += ctx->slices[i].ret < 0;
 
     if (error)
-        av_frame_set_decode_error_flags(ctx->frame, FF_DECODE_ERROR_INVALID_BITSTREAM);
+        ctx->frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
     if (error < ctx->slice_count)
         return 0;
 
diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
index e53eb2e..cf2d2e0 100644
--- a/libavcodec/rawdec.c
+++ b/libavcodec/rawdec.c
@@ -237,8 +237,8 @@  static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame,
     if (res < 0)
         return res;
 
-    av_frame_set_pkt_pos     (frame, avctx->internal->pkt->pos);
-    av_frame_set_pkt_duration(frame, avctx->internal->pkt->duration);
+    frame->pkt_pos      = avctx->internal->pkt->pos;
+    frame->pkt_duration = avctx->internal->pkt->duration;
 
     if (context->tff >= 0) {
         frame->interlaced_frame = 1;
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index cb1a34e..c8e24e3 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -227,9 +227,9 @@  static int add_metadata(int count, int type,
                         const char *name, const char *sep, TiffContext *s, AVFrame *frame)
 {
     switch(type) {
-    case TIFF_DOUBLE: return ff_tadd_doubles_metadata(count, name, sep, &s->gb, s->le, avpriv_frame_get_metadatap(frame));
-    case TIFF_SHORT : return ff_tadd_shorts_metadata(count, name, sep, &s->gb, s->le, 0, avpriv_frame_get_metadatap(frame));
-    case TIFF_STRING: return ff_tadd_string_metadata(count, name, &s->gb, s->le, avpriv_frame_get_metadatap(frame));
+    case TIFF_DOUBLE: return ff_tadd_doubles_metadata(count, name, sep, &s->gb, s->le, &frame->metadata);
+    case TIFF_SHORT : return ff_tadd_shorts_metadata(count, name, sep, &s->gb, s->le, 0, &frame->metadata);
+    case TIFF_STRING: return ff_tadd_string_metadata(count, name, &s->gb, s->le, &frame->metadata);
     default         : return AVERROR_INVALIDDATA;
     };
 }
@@ -1255,7 +1255,7 @@  static int decode_frame(AVCodecContext *avctx,
             av_log(avctx, AV_LOG_WARNING, "Type of GeoTIFF key %d is wrong\n", s->geotags[i].key);
             continue;
         }
-        ret = av_dict_set(avpriv_frame_get_metadatap(p), keyname, s->geotags[i].val, 0);
+        ret = av_dict_set(&p->metadata, keyname, s->geotags[i].val, 0);
         if (ret<0) {
             av_log(avctx, AV_LOG_ERROR, "Writing metadata with key '%s' failed\n", keyname);
             return ret;
diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index 45abfdc..52a8040 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1489,7 +1489,7 @@  static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
                 goto exif_end;
             }
 
-            av_dict_copy(avpriv_frame_get_metadatap(data), exif_metadata, 0);
+            av_dict_copy(&((AVFrame *) data)->metadata, exif_metadata, 0);
 
 exif_end:
             av_dict_free(&exif_metadata);