diff mbox series

[FFmpeg-devel,2/4] doc/examples/muxing.c: Fixed a compile warning

Message ID bdc58ac63bbf5bd2933d3210fa726a963b88159b.1588413324.git.gang.zhao.42@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4] doc/examples/demuxing_decoding: Fixed a compile warning
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Zhao, Gang May 2, 2020, 10:10 a.m. UTC
Fixed deprecation declarations compile warning by replacing deprecated
api avcodec_encode_{audio,video}2() with
avcodec_send_frame()/avcodec_receive_packet(). Also made some clean up
according to this change.

Signed-off-by: Zhao, Gang <gang.zhao.42@gmail.com>
---
Tested that the new code generated the same file as before by running
./doc/examples/muxing_g my_samples/muxing.mp4

 doc/examples/muxing.c | 104 +++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 67 deletions(-)

Comments

Nicolas George May 2, 2020, 10:13 a.m. UTC | #1
Zhao, Gang (12020-05-02):
> Fixed deprecation declarations compile warning by replacing deprecated
> api avcodec_encode_{audio,video}2() with
> avcodec_send_frame()/avcodec_receive_packet(). Also made some clean up
> according to this change.

That is not "fixing a warning", that is changing the whole logic. At the
very least, th commit message should say that.

But your changes seemed a little more invasive than that, for example
renaming functions.

Regards,
Zhao, Gang May 2, 2020, 12:36 p.m. UTC | #2
On Sat, May 2, 2020 at 6:13 PM Nicolas George <george@nsup.org> wrote:

> Zhao, Gang (12020-05-02):
> > Fixed deprecation declarations compile warning by replacing deprecated
> > api avcodec_encode_{audio,video}2() with
> > avcodec_send_frame()/avcodec_receive_packet(). Also made some clean up
> > according to this change.
>
> That is not "fixing a warning", that is changing the whole logic. At the
> very least, th commit message should say that.
>
> I suggested a new commit log in reply to your comment in patch 01.


> But your changes seemed a little more invasive than that, for example
> renaming functions.


With the new api, function write_audio_frame and write_video_frame can be
combined to one.
Because there is already a local function called write_frame, I changed the
name to muxing_write_frame
to avoid name collision.

>
>
Regards,
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Nicolas George May 2, 2020, 12:39 p.m. UTC | #3
Zhao, Gang (12020-05-02):
> With the new api, function write_audio_frame and write_video_frame can be
> combined to one.
> Because there is already a local function called write_frame, I changed the
> name to muxing_write_frame
> to avoid name collision.

Multiple changes in the same patch make it harder to review. You could
name the merged function write_any_frame().

And if you think it is best, make another patch to rename both
functions: it would be easier to review.

Regards,
diff mbox series

Patch

diff --git doc/examples/muxing.c doc/examples/muxing.c
index 9af9aae483..7e781c0783 100644
--- doc/examples/muxing.c
+++ doc/examples/muxing.c
@@ -280,8 +280,10 @@  static void open_audio(AVFormatContext *oc, AVCodec *codec, OutputStream *ost, A
 static AVFrame *get_audio_frame(OutputStream *ost)
 {
     AVFrame *frame = ost->tmp_frame;
-    int j, i, v;
+    int j, i, v, ret;
     int16_t *q = (int16_t*)frame->data[0];
+    int dst_nb_samples;
+    AVCodecContext *c = ost->enc;
 
     /* check if we want to generate more frames */
     if (av_compare_ts(ost->next_pts, ost->enc->time_base,
@@ -299,27 +301,6 @@  static AVFrame *get_audio_frame(OutputStream *ost)
     frame->pts = ost->next_pts;
     ost->next_pts  += frame->nb_samples;
 
-    return frame;
-}
-
-/*
- * encode one audio frame and send it to the muxer
- * return 1 when encoding is finished, 0 otherwise
- */
-static int write_audio_frame(AVFormatContext *oc, OutputStream *ost)
-{
-    AVCodecContext *c;
-    AVPacket pkt = { 0 }; // data and size must be 0;
-    AVFrame *frame;
-    int ret;
-    int got_packet;
-    int dst_nb_samples;
-
-    av_init_packet(&pkt);
-    c = ost->enc;
-
-    frame = get_audio_frame(ost);
-
     if (frame) {
         /* convert samples from native format to destination codec format, using the resampler */
             /* compute destination number of samples */
@@ -349,13 +330,40 @@  static int write_audio_frame(AVFormatContext *oc, OutputStream *ost)
         ost->samples_count += dst_nb_samples;
     }
 
-    ret = avcodec_encode_audio2(c, &pkt, frame, &got_packet);
+    return frame;
+}
+
+/*
+ * encode one frame and send it to the muxer
+ * return 1 when encoding is finished, 0 otherwise
+ */
+static int muxing_write_frame(AVFormatContext *oc, OutputStream *ost, AVFrame *frame)
+{
+    AVCodecContext *c;
+    AVPacket pkt = { 0 }; // data and size must be 0;
+    int ret;
+
+    av_init_packet(&pkt);
+    c = ost->enc;
+
+    /* send the frame for encoding */
+    ret = avcodec_send_frame(c, frame);
     if (ret < 0) {
-        fprintf(stderr, "Error encoding audio frame: %s\n", av_err2str(ret));
+        fprintf(stderr, "Error sending the frame to the encoder: %s\n", av_err2str(ret));
         exit(1);
     }
 
-    if (got_packet) {
+    /* read all the available output packets (in general there may be any
+     * number of them */
+    while (ret >= 0) {
+        ret = avcodec_receive_packet(c, &pkt);
+        if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
+            goto exit;
+        else if (ret < 0) {
+            fprintf(stderr, "Error encoding audio frame\n");
+            exit(1);
+        }
+
         ret = write_frame(oc, &c->time_base, ost->st, &pkt);
         if (ret < 0) {
             fprintf(stderr, "Error while writing audio frame: %s\n",
@@ -364,7 +372,8 @@  static int write_audio_frame(AVFormatContext *oc, OutputStream *ost)
         }
     }
 
-    return (frame || got_packet) ? 0 : 1;
+exit:
+    return frame ? 0 : 1;
 }
 
 /**************************************************************/
@@ -500,45 +509,6 @@  static AVFrame *get_video_frame(OutputStream *ost)
     return ost->frame;
 }
 
-/*
- * encode one video frame and send it to the muxer
- * return 1 when encoding is finished, 0 otherwise
- */
-static int write_video_frame(AVFormatContext *oc, OutputStream *ost)
-{
-    int ret;
-    AVCodecContext *c;
-    AVFrame *frame;
-    int got_packet = 0;
-    AVPacket pkt = { 0 };
-
-    c = ost->enc;
-
-    frame = get_video_frame(ost);
-
-    av_init_packet(&pkt);
-
-    /* encode the image */
-    ret = avcodec_encode_video2(c, &pkt, frame, &got_packet);
-    if (ret < 0) {
-        fprintf(stderr, "Error encoding video frame: %s\n", av_err2str(ret));
-        exit(1);
-    }
-
-    if (got_packet) {
-        ret = write_frame(oc, &c->time_base, ost->st, &pkt);
-    } else {
-        ret = 0;
-    }
-
-    if (ret < 0) {
-        fprintf(stderr, "Error while writing video frame: %s\n", av_err2str(ret));
-        exit(1);
-    }
-
-    return (frame || got_packet) ? 0 : 1;
-}
-
 static void close_stream(AVFormatContext *oc, OutputStream *ost)
 {
     avcodec_free_context(&ost->enc);
@@ -638,9 +608,9 @@  int main(int argc, char **argv)
         if (encode_video &&
             (!encode_audio || av_compare_ts(video_st.next_pts, video_st.enc->time_base,
                                             audio_st.next_pts, audio_st.enc->time_base) <= 0)) {
-            encode_video = !write_video_frame(oc, &video_st);
+            encode_video = !muxing_write_frame(oc, &video_st, get_video_frame(&video_st));
         } else {
-            encode_audio = !write_audio_frame(oc, &audio_st);
+            encode_audio = !muxing_write_frame(oc, &audio_st, get_audio_frame(&audio_st));
         }
     }