[FFmpeg-devel] [PATCH 0/2] Harmonise comments about ownership/meaning

[FFmpeg-devel] [PATCH 0/2] Harmonise comments about ownership/meaning

[Andrew Sayers]

There seems to be a couple of documentation conventions in the code:

  /**
   * - encoding: (who sets this in encoding context)
   * - decoding: (who sets this in decoding context)
   */
  int foo;

  /**
   * Encoding: (meaning in encoding context)
   * Decoding: (meaning in decoding context)
   */
  int bar;

These are very useful, but the layout causes some issues:

1. (slightly) different layouts are used in some places
2. doxygen renders the "meaning" layout as one big paragraph, which is hard to read
3. the layouts are quite similar, so it can be hard to tell at a glance
   whether a given piece of documentation is talking about ownership or meaning

For a good example of this issue, please see [1].

Harmonise layouts and propose a new layout for meanings:

  /**
   * *Encoding:*
   *
   *   Meaning in encoding context
   *
   * *Decoding:*
   *
   *   Meaning in decoding context
   */

I hope this isn't too forward for my second day around here, but IMHO
this would significantly improve readability.

[1] https://ffmpeg.org/doxygen/6.0/structAVCodecContext.html#a948993adfdfcd64b81dad1151fe50f33

Andrew Sayers (2):
      avformat: harmonise "- {decoding,encoding,demuxing,muxing}: " comments
      all: harmonise "{Decoding,Encoding,Audio,Video}:" comments

 libavcodec/avcodec.h   | 19 +++++++----
 libavformat/avformat.h | 89 +++++++++++++++++++++++++++-----------------------
 2 files changed, 62 insertions(+), 46 deletions(-)


_______________________________________________
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".

[FFmpeg-devel] [PATCH 1/2] avformat: harmonise "- {decoding, encoding, demuxing, muxing}: " comments

[Andrew Sayers]

There seems to be a convention for documenting ownership:

    /**
     * - encoding: (who sets this in encoding context)
     * - decoding: (who sets this in decoding context)
     */
    int foo;

Ensure all such comments start with a "-" and use lower case,
so doxygen formats them as a bulleted lists instead of one
long paragraph.

Signed-off-by: Andrew Sayers <ffmpeg-devel@pileofstuff.org>
---
 libavformat/avformat.h | 67 +++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index f4506f4cf1..35b7f78ec7 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -849,8 +849,8 @@ typedef struct AVStream {
     int index;    /**< stream index in AVFormatContext */
     /**
      * Format-specific stream ID.
-     * decoding: set by libavformat
-     * encoding: set by the user, replaced by libavformat if left unset
+     * - decoding: set by libavformat
+     * - encoding: set by the user, replaced by libavformat if left unset
      */
     int id;
 
@@ -871,13 +871,13 @@ typedef struct AVStream {
      * This is the fundamental unit of time (in seconds) in terms
      * of which frame timestamps are represented.
      *
-     * decoding: set by libavformat
-     * encoding: May be set by the caller before avformat_write_header() to
-     *           provide a hint to the muxer about the desired timebase. In
-     *           avformat_write_header(), the muxer will overwrite this field
-     *           with the timebase that will actually be used for the timestamps
-     *           written into the file (which may or may not be related to the
-     *           user-provided one, depending on the format).
+     * - decoding: set by libavformat
+     * - encoding: May be set by the caller before avformat_write_header() to
+     *             provide a hint to the muxer about the desired timebase. In
+     *             avformat_write_header(), the muxer will overwrite this field
+     *             with the timebase that will actually be used for the timestamps
+     *             written into the file (which may or may not be related to the
+     *             user-provided one, depending on the format).
      */
     AVRational time_base;
 
@@ -896,8 +896,9 @@ typedef struct AVStream {
      * If a source file does not specify a duration, but does specify
      * a bitrate, this value will be estimated from bitrate and file size.
      *
-     * Encoding: May be set by the caller before avformat_write_header() to
-     * provide a hint to the muxer about the estimated duration.
+     * - decoding: set by libavformat
+     * - encoding: may be set by the caller before avformat_write_header() to
+     *             provide a hint to the muxer about the estimated duration.
      */
     int64_t duration;
 
@@ -935,8 +936,8 @@ typedef struct AVStream {
      * For streams with AV_DISPOSITION_ATTACHED_PIC disposition, this packet
      * will contain the attached picture.
      *
-     * decoding: set by libavformat, must not be modified by the caller.
-     * encoding: unused
+     * - decoding: set by libavformat, must not be modified by the caller.
+     * - encoding: unused
      */
     AVPacket attached_pic;
 
@@ -1203,16 +1204,16 @@ typedef struct AVStreamGroup {
     /**
      * Group type-specific group ID.
      *
-     * decoding: set by libavformat
-     * encoding: may set by the user
+     * - decoding: set by libavformat
+     * - encoding: may set by the user
      */
     int64_t id;
 
     /**
      * Group type
      *
-     * decoding: set by libavformat on group creation
-     * encoding: set by avformat_stream_group_create()
+     * - decoding: set by libavformat on group creation
+     * - encoding: set by avformat_stream_group_create()
      */
     enum AVStreamGroupParamsType type;
 
@@ -1534,19 +1535,19 @@ typedef struct AVFormatContext {
 
     /**
      * Forced video codec_id.
-     * Demuxing: Set by user.
+     * - demuxing: Set by user.
      */
     enum AVCodecID video_codec_id;
 
     /**
      * Forced audio codec_id.
-     * Demuxing: Set by user.
+     * - demuxing: Set by user.
      */
     enum AVCodecID audio_codec_id;
 
     /**
      * Forced subtitle codec_id.
-     * Demuxing: Set by user.
+     * - demuxing: Set by user.
      */
     enum AVCodecID subtitle_codec_id;
 
@@ -1622,11 +1623,11 @@ typedef struct AVFormatContext {
     /**
      * Custom interrupt callbacks for the I/O layer.
      *
-     * demuxing: set by the user before avformat_open_input().
-     * muxing: set by the user before avformat_write_header()
-     * (mainly useful for AVFMT_NOFILE formats). The callback
-     * should also be passed to avio_open2() if it's used to
-     * open the file.
+     * - demuxing: set by the user before avformat_open_input().
+     * - muxing: set by the user before avformat_write_header()
+     *           (mainly useful for AVFMT_NOFILE formats). The callback
+     *           should also be passed to avio_open2() if it's used to
+     *           open the file.
      */
     AVIOInterruptCB interrupt_callback;
 
@@ -1828,7 +1829,7 @@ typedef struct AVFormatContext {
      * Forced video codec.
      * This allows forcing a specific decoder, even when there are multiple with
      * the same codec_id.
-     * Demuxing: Set by user
+     * - demuxing: Set by user
      */
     const struct AVCodec *video_codec;
 
@@ -1836,7 +1837,7 @@ typedef struct AVFormatContext {
      * Forced audio codec.
      * This allows forcing a specific decoder, even when there are multiple with
      * the same codec_id.
-     * Demuxing: Set by user
+     * - demuxing: Set by user
      */
     const struct AVCodec *audio_codec;
 
@@ -1844,7 +1845,7 @@ typedef struct AVFormatContext {
      * Forced subtitle codec.
      * This allows forcing a specific decoder, even when there are multiple with
      * the same codec_id.
-     * Demuxing: Set by user
+     * - demuxing: Set by user
      */
     const struct AVCodec *subtitle_codec;
 
@@ -1852,14 +1853,14 @@ typedef struct AVFormatContext {
      * Forced data codec.
      * This allows forcing a specific decoder, even when there are multiple with
      * the same codec_id.
-     * Demuxing: Set by user
+     * - demuxing: Set by user
      */
     const struct AVCodec *data_codec;
 
     /**
      * Number of bytes to be written as padding in a metadata header.
-     * Demuxing: Unused.
-     * Muxing: Set by user.
+     * - demuxing: Unused.
+     * - muxing: Set by user.
      */
     int metadata_header_padding;
 
@@ -1876,7 +1877,7 @@ typedef struct AVFormatContext {
 
     /**
      * Output timestamp offset, in microseconds.
-     * Muxing: set by user
+     * - muxing: set by user
      */
     int64_t output_ts_offset;
 
@@ -1890,7 +1891,7 @@ typedef struct AVFormatContext {
 
     /**
      * Forced Data codec_id.
-     * Demuxing: Set by user.
+     * - demuxing: Set by user.
      */
     enum AVCodecID data_codec_id;
 
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat: harmonise "- {decoding, encoding, demuxing, muxing}: " comments

[Stefano Sabatini]

Hi, sorry for the slow reply.

On date Thursday 2024-02-29 16:23:06 +0000, Andrew Sayers wrote:

> There seems to be a convention for documenting ownership:

I find "ownership" a bit confusing, probably it's better to talk about
context usage.

> 
>     /**
>      * - encoding: (who sets this in encoding context)
>      * - decoding: (who sets this in decoding context)
>      */
>     int foo;

> 
> Ensure all such comments start with a "-" and use lower case,
> so doxygen formats them as a bulleted lists instead of one
> long paragraph.
> 
> Signed-off-by: Andrew Sayers <ffmpeg-devel@pileofstuff.org>
> ---
>  libavformat/avformat.h | 67 +++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index f4506f4cf1..35b7f78ec7 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -849,8 +849,8 @@ typedef struct AVStream {
>      int index;    /**< stream index in AVFormatContext */
>      /**
>       * Format-specific stream ID.
> -     * decoding: set by libavformat
> -     * encoding: set by the user, replaced by libavformat if left unset
> +     * - decoding: set by libavformat
> +     * - encoding: set by the user, replaced by libavformat if left unset
>       */
>      int id;
>  
> @@ -871,13 +871,13 @@ typedef struct AVStream {
>       * This is the fundamental unit of time (in seconds) in terms
>       * of which frame timestamps are represented.
>       *
> -     * decoding: set by libavformat
> -     * encoding: May be set by the caller before avformat_write_header() to
> -     *           provide a hint to the muxer about the desired timebase. In
> -     *           avformat_write_header(), the muxer will overwrite this field
> -     *           with the timebase that will actually be used for the timestamps
> -     *           written into the file (which may or may not be related to the
> -     *           user-provided one, depending on the format).
> +     * - decoding: set by libavformat
> +     * - encoding: May be set by the caller before avformat_write_header() to
> +     *             provide a hint to the muxer about the desired timebase. In
> +     *             avformat_write_header(), the muxer will overwrite this field
> +     *             with the timebase that will actually be used for the timestamps
> +     *             written into the file (which may or may not be related to the
> +     *             user-provided one, depending on the format).
>       */
>      AVRational time_base;
>  
> @@ -896,8 +896,9 @@ typedef struct AVStream {
>       * If a source file does not specify a duration, but does specify
>       * a bitrate, this value will be estimated from bitrate and file size.
>       *
> -     * Encoding: May be set by the caller before avformat_write_header() to
> -     * provide a hint to the muxer about the estimated duration.
> +     * - decoding: set by libavformat
> +     * - encoding: may be set by the caller before avformat_write_header() to
> +     *             provide a hint to the muxer about the estimated duration.
>       */
>      int64_t duration;
>  
> @@ -935,8 +936,8 @@ typedef struct AVStream {
>       * For streams with AV_DISPOSITION_ATTACHED_PIC disposition, this packet
>       * will contain the attached picture.
>       *
> -     * decoding: set by libavformat, must not be modified by the caller.
> -     * encoding: unused
> +     * - decoding: set by libavformat, must not be modified by the caller.
> +     * - encoding: unused
>       */
>      AVPacket attached_pic;
>  
> @@ -1203,16 +1204,16 @@ typedef struct AVStreamGroup {
>      /**
>       * Group type-specific group ID.
>       *
> -     * decoding: set by libavformat
> -     * encoding: may set by the user
> +     * - decoding: set by libavformat
> +     * - encoding: may set by the user
>       */
>      int64_t id;
>  
>      /**
>       * Group type
>       *
> -     * decoding: set by libavformat on group creation
> -     * encoding: set by avformat_stream_group_create()
> +     * - decoding: set by libavformat on group creation
> +     * - encoding: set by avformat_stream_group_create()
>       */
>      enum AVStreamGroupParamsType type;
>  
> @@ -1534,19 +1535,19 @@ typedef struct AVFormatContext {
>  
>      /**
>       * Forced video codec_id.

> -     * Demuxing: Set by user.
> +     * - demuxing: Set by user.

while at it, probably we should use "decoding" in place of demuxing,
since in this file "decoding" is semantically equivalent and used most
prominently, same below

[...]
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat: harmonise "- {decoding, encoding, demuxing, muxing}: " comments

[Andrew Sayers]

Hey, no problem :)

On 09/03/2024 16:06, Stefano Sabatini wrote:
> Hi, sorry for the slow reply.
> 
> On date Thursday 2024-02-29 16:23:06 +0000, Andrew Sayers wrote:
> 
>> There seems to be a convention for documenting ownership:
> 
> I find "ownership" a bit confusing, probably it's better to talk about
> context usage.

On reflection, it might be better to borrow some terms from function 
documentation.  So s/ownership/direction/ and s/meaning/description/

Here's a silly example of a well-documented function and equivalent 
struct using the proposed layout:

/**
  *
  * @brief AI-aging magic (functional version)
  *
  * @param[in,out] age   AI-generated age value
  *
  * - encoding: in
  * - decoding: out
  *
  * *Video encoding*
  *
  * Run all faces through a filter that makes them look
  * this many years old.
  *
  * Values outside of the range 1-100 will be ignored.
  *
  * *Audio decoding*
  *
  * Indicates the age of the primary speaker in years.
  * May be less accurate for non-English speakers.
  *
  * Values <0 indicate the age could not be determined.
  */
void example_function( ExampleContext* ctx, int age );

/**
  * @brief AI-aging magic (structural version)
  */
struct ExampleContext {

     /**
      *
      * @brief AI-generated age value
      *
      * - encoding: set by user
      * - decoding: set by libav
      *
      * *Video encoding*
      *
      * Run all faces through a filter that makes them look
      * this many years old.
      *
      * Values outside of the range 1-100 will be ignored.
      *
      * *Audio decoding*
      *
      * Indicates the age of the primary speaker in years.
      * May be less accurate for non-English speakers.
      *
      * Values <0 indicate the age could not be determined.
      */
     int age;

};

As a reader, I want to know how I'm supposed to engage with a variable 
(direction) before I can understand the deeper semantics (description). 
Representing the former with a bulleted list makes it easier to glance 
through the docs and find the variable I'm looking for, while 
representing the latter with headings and paragraphs makes it easier to 
focus on the bit I care about right now.

> 
>>
>>      /**
>>       * - encoding: (who sets this in encoding context)
>>       * - decoding: (who sets this in decoding context)
>>       */
>>      int foo;
> 
>>
>> Ensure all such comments start with a "-" and use lower case,
>> so doxygen formats them as a bulleted lists instead of one
>> long paragraph.
>>
>> Signed-off-by: Andrew Sayers <ffmpeg-devel@pileofstuff.org>
>> ---
>>   libavformat/avformat.h | 67 +++++++++++++++++++++---------------------
>>   1 file changed, 34 insertions(+), 33 deletions(-)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index f4506f4cf1..35b7f78ec7 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -849,8 +849,8 @@ typedef struct AVStream {
>>       int index;    /**< stream index in AVFormatContext */
>>       /**
>>        * Format-specific stream ID.
>> -     * decoding: set by libavformat
>> -     * encoding: set by the user, replaced by libavformat if left unset
>> +     * - decoding: set by libavformat
>> +     * - encoding: set by the user, replaced by libavformat if left unset
>>        */
>>       int id;
>>   
>> @@ -871,13 +871,13 @@ typedef struct AVStream {
>>        * This is the fundamental unit of time (in seconds) in terms
>>        * of which frame timestamps are represented.
>>        *
>> -     * decoding: set by libavformat
>> -     * encoding: May be set by the caller before avformat_write_header() to
>> -     *           provide a hint to the muxer about the desired timebase. In
>> -     *           avformat_write_header(), the muxer will overwrite this field
>> -     *           with the timebase that will actually be used for the timestamps
>> -     *           written into the file (which may or may not be related to the
>> -     *           user-provided one, depending on the format).
>> +     * - decoding: set by libavformat
>> +     * - encoding: May be set by the caller before avformat_write_header() to
>> +     *             provide a hint to the muxer about the desired timebase. In
>> +     *             avformat_write_header(), the muxer will overwrite this field
>> +     *             with the timebase that will actually be used for the timestamps
>> +     *             written into the file (which may or may not be related to the
>> +     *             user-provided one, depending on the format).
>>        */
>>       AVRational time_base;
>>   
>> @@ -896,8 +896,9 @@ typedef struct AVStream {
>>        * If a source file does not specify a duration, but does specify
>>        * a bitrate, this value will be estimated from bitrate and file size.
>>        *
>> -     * Encoding: May be set by the caller before avformat_write_header() to
>> -     * provide a hint to the muxer about the estimated duration.
>> +     * - decoding: set by libavformat
>> +     * - encoding: may be set by the caller before avformat_write_header() to
>> +     *             provide a hint to the muxer about the estimated duration.
>>        */
>>       int64_t duration;
>>   
>> @@ -935,8 +936,8 @@ typedef struct AVStream {
>>        * For streams with AV_DISPOSITION_ATTACHED_PIC disposition, this packet
>>        * will contain the attached picture.
>>        *
>> -     * decoding: set by libavformat, must not be modified by the caller.
>> -     * encoding: unused
>> +     * - decoding: set by libavformat, must not be modified by the caller.
>> +     * - encoding: unused
>>        */
>>       AVPacket attached_pic;
>>   
>> @@ -1203,16 +1204,16 @@ typedef struct AVStreamGroup {
>>       /**
>>        * Group type-specific group ID.
>>        *
>> -     * decoding: set by libavformat
>> -     * encoding: may set by the user
>> +     * - decoding: set by libavformat
>> +     * - encoding: may set by the user
>>        */
>>       int64_t id;
>>   
>>       /**
>>        * Group type
>>        *
>> -     * decoding: set by libavformat on group creation
>> -     * encoding: set by avformat_stream_group_create()
>> +     * - decoding: set by libavformat on group creation
>> +     * - encoding: set by avformat_stream_group_create()
>>        */
>>       enum AVStreamGroupParamsType type;
>>   
>> @@ -1534,19 +1535,19 @@ typedef struct AVFormatContext {
>>   
>>       /**
>>        * Forced video codec_id.
> 
>> -     * Demuxing: Set by user.
>> +     * - demuxing: Set by user.
> 
> while at it, probably we should use "decoding" in place of demuxing,
> since in this file "decoding" is semantically equivalent and used most
> prominently, same below

My only immediate opinion about "decoding" is that it should go in a 
separate patch.  Functional vs. cosmetic changes and all that.

> 
> [...]
> _______________________________________________
> 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".
> 
_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2] all: harmonise "{Decoding, Encoding, Audio, Video}:" comments

[Andrew Sayers]

There seems to be a convention for documenting meanings:

    /**
     * Encoding: (meaning in encoding context)
     * Decoding: (meaning in decoding context)
     */

At a glance, these are confusingly similar to ownership comments.
They are also rendered by doxygen as long, hard-to-read paragraphs.

Reformat these comments to be more visually distinct and readable.
---
 libavcodec/avcodec.h   | 19 +++++++++++++------
 libavformat/avformat.h | 22 +++++++++++++++-------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 43859251cc..e2193f1d00 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -586,16 +586,23 @@ typedef struct AVCodecContext {
     /**
      * Codec delay.
      *
-     * Encoding: Number of frames delay there will be from the encoder input to
-     *           the decoder output. (we assume the decoder matches the spec)
-     * Decoding: Number of frames delay in addition to what a standard decoder
-     *           as specified in the spec would produce.
+     * *Encoding:*
+     *
+     *   Number of frames delay there will be from the encoder input to
+     *   the decoder output. (we assume the decoder matches the spec).
+     *
+     * *Decoding:*
+     *
+     *   Number of frames delay in addition to what a standard decoder
+     *   as specified in the spec would produce.
+     *
+     * *Video:*
      *
-     * Video:
      *   Number of frames the decoded output will be delayed relative to the
      *   encoded input.
      *
-     * Audio:
+     * *Audio:*
+     *
      *   For encoding, this field is unused (see initial_padding).
      *
      *   For decoding, this is the number of samples the decoder needs to
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 35b7f78ec7..be97947bd1 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -882,19 +882,27 @@ typedef struct AVStream {
     AVRational time_base;
 
     /**
-     * Decoding: pts of the first frame of the stream in presentation order, in stream time base.
-     * Only set this if you are absolutely 100% sure that the value you set
-     * it to really is the pts of the first frame.
-     * This may be undefined (AV_NOPTS_VALUE).
+     * *Decoding:*
+     *
+     *   pts of the first frame of the stream in presentation order, in stream time base.
+     *
+     *   Only set this if you are absolutely 100% sure that the value you set
+     *   it to really is the pts of the first frame.
+     *
+     *   This may be undefined (AV_NOPTS_VALUE).
+     *
      * @note The ASF header does NOT contain a correct start_time the ASF
      * demuxer must NOT set this.
      */
     int64_t start_time;
 
     /**
-     * Decoding: duration of the stream, in stream time base.
-     * If a source file does not specify a duration, but does specify
-     * a bitrate, this value will be estimated from bitrate and file size.
+     * *Decoding:*
+     *
+     *   Duration of the stream, in stream time base.
+     *
+     *   If a source file does not specify a duration, but does specify
+     *   a bitrate, this value will be estimated from bitrate and file size.
      *
      * - decoding: set by libavformat
      * - encoding: may be set by the caller before avformat_write_header() to
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] all: harmonise "{Decoding, Encoding, Audio, Video}:" comments

[Stefano Sabatini]

On date Thursday 2024-02-29 16:23:07 +0000, Andrew Sayers wrote:

> There seems to be a convention for documenting meanings:
> 
>     /**
>      * Encoding: (meaning in encoding context)
>      * Decoding: (meaning in decoding context)
>      */
> 
> At a glance, these are confusingly similar to ownership comments.
> They are also rendered by doxygen as long, hard-to-read paragraphs.

I still don't understand what you mean by "ownership" and "meaning"
and their difference.

To me they look like the same thing, i.e. they define the semantics
depending on the operation context (encoding/decoding). In addition to
this, there might be a difference releated to the media content. To
distinguish the two, I'd say "operational" and "media" context.

> Reformat these comments to be more visually distinct and readable.
> ---
>  libavcodec/avcodec.h   | 19 +++++++++++++------
>  libavformat/avformat.h | 22 +++++++++++++++-------
>  2 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 43859251cc..e2193f1d00 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -586,16 +586,23 @@ typedef struct AVCodecContext {
>      /**
>       * Codec delay.
>       *
> -     * Encoding: Number of frames delay there will be from the encoder input to
> -     *           the decoder output. (we assume the decoder matches the spec)
> -     * Decoding: Number of frames delay in addition to what a standard decoder
> -     *           as specified in the spec would produce.
> +     * *Encoding:*
> +     *
> +     *   Number of frames delay there will be from the encoder input to
> +     *   the decoder output. (we assume the decoder matches the spec).
> +     *
> +     * *Decoding:*
> +     *
> +     *   Number of frames delay in addition to what a standard decoder
> +     *   as specified in the spec would produce.
> +     *
> +     * *Video:*
>       *
> -     * Video:
>       *   Number of frames the decoded output will be delayed relative to the
>       *   encoded input.
>       *
> -     * Audio:
> +     * *Audio:*
> +     *
>       *   For encoding, this field is unused (see initial_padding).
>       *
>       *   For decoding, this is the number of samples the decoder needs to
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 35b7f78ec7..be97947bd1 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -882,19 +882,27 @@ typedef struct AVStream {
>      AVRational time_base;
>  
>      /**
> -     * Decoding: pts of the first frame of the stream in presentation order, in stream time base.
> -     * Only set this if you are absolutely 100% sure that the value you set
> -     * it to really is the pts of the first frame.
> -     * This may be undefined (AV_NOPTS_VALUE).
> +     * *Decoding:*
> +     *
> +     *   pts of the first frame of the stream in presentation order, in stream time base.
> +     *
> +     *   Only set this if you are absolutely 100% sure that the value you set
> +     *   it to really is the pts of the first frame.
> +     *
> +     *   This may be undefined (AV_NOPTS_VALUE).
> +     *

This conventions seems at odd with the prevoius patch, where you was
preferring the - decoding/- encoding style.

[...]
_______________________________________________
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".