Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
Quoting Soft Works (2022-06-05 07:23:18) > This is causing a regression in ffprobe. > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which > was required for ffprobe and had been added with > e83c716e16c52fa56a78274408f7628e5dc719da. > > The demand from the commit message is not yet guaranteed to be fulfilled: > > > On entry to avcodec_open2(), the type and id either have to be > > UNKNOWN/NONE or have to match the codec to be used. > > I have one verified example (maybe a second will follow), which is an MKV with > an attachment "stream" of type "text". > The found codec will be textdec of type 'subtitle' even though the stream type > is attachment. Without the special condition for attachment streams, this > is now causing ffprobe to error out with non-zero exit code and incomplete > output. > > > > Example: > > [...] > Stream #0:9: Attachment: text > Metadata: > filename: textfile.text > mimetype: text/plain > [text @ 01AC32310340] Codec type or id mismatches > Could not open codec for input stream 9 > This sounds very much like a bug in ffprobe. It makes no sense to call avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 9:01 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > Quoting Soft Works (2022-06-05 07:23:18) > > This is causing a regression in ffprobe. > > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which > > was required for ffprobe and had been added with > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > The demand from the commit message is not yet guaranteed to be fulfilled: > > > > > On entry to avcodec_open2(), the type and id either have to be > > > UNKNOWN/NONE or have to match the codec to be used. > > > > I have one verified example (maybe a second will follow), which is an MKV > with > > an attachment "stream" of type "text". > > The found codec will be textdec of type 'subtitle' even though the stream > type > > is attachment. Without the special condition for attachment streams, this > > is now causing ffprobe to error out with non-zero exit code and incomplete > > output. > > > > > > > > Example: > > > > [...] > > Stream #0:9: Attachment: text > > Metadata: > > filename: textfile.text > > mimetype: text/plain > > [text @ 01AC32310340] Codec type or id mismatches > > Could not open codec for input stream 9 > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. You make a behavioral change to an API function that had this behavior established and constant over more than 10 years, and when that change breaks functionality, it's the callers' fault? How does this go together with all that peanut counting of major, minor and micro version numbers per library? What is this versioning good for, when you can make breaking changes and declare the breakage as bugs? Though, I don't want to say that your change is wrong or shouldn't be made. Yet, the change requires ffprobe to be adjusted (I just wouldn't call it a "bug fix"..) Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Sunday, June 5, 2022 9:55 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Anton > > Khirnov > > Sent: Sunday, June 5, 2022 9:01 AM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec > id/type > > validity checking > > > > Quoting Soft Works (2022-06-05 07:23:18) > > > This is causing a regression in ffprobe. > > > > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT > which > > > was required for ffprobe and had been added with > > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > > > The demand from the commit message is not yet guaranteed to be fulfilled: > > > > > > > On entry to avcodec_open2(), the type and id either have to be > > > > UNKNOWN/NONE or have to match the codec to be used. > > > > > > I have one verified example (maybe a second will follow), which is an MKV > > with > > > an attachment "stream" of type "text". > > > The found codec will be textdec of type 'subtitle' even though the stream > > type > > > is attachment. Without the special condition for attachment streams, this > > > is now causing ffprobe to error out with non-zero exit code and > incomplete > > > output. > > > > > > > > > > > > Example: > > > > > > [...] > > > Stream #0:9: Attachment: text > > > Metadata: > > > filename: textfile.text > > > mimetype: text/plain > > > [text @ 01AC32310340] Codec type or id mismatches > > > Could not open codec for input stream 9 > > > > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. > > You make a behavioral change to an API function that had this behavior > established and constant over more than 10 years, and when that change > breaks functionality, it's the callers' fault? > How does this go together with all that peanut counting of major, minor > and micro version numbers per library? What is this versioning good for, > when you can make breaking changes and declare the breakage as bugs? As per your logic - wouldn't this change require a major version bump? Or even an avcodec_open3() to ensure backward-compatibility? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
Quoting Soft Works (2022-06-05 09:54:51) > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Anton > > Khirnov > > Sent: Sunday, June 5, 2022 9:01 AM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > > validity checking > > > > Quoting Soft Works (2022-06-05 07:23:18) > > > This is causing a regression in ffprobe. > > > > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT > > > which > > > was required for ffprobe and had been added with > > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > > > The demand from the commit message is not yet guaranteed to be fulfilled: > > > > > > > On entry to avcodec_open2(), the type and id either have to be > > > > UNKNOWN/NONE or have to match the codec to be used. > > > > > > I have one verified example (maybe a second will follow), which is an MKV > > with > > > an attachment "stream" of type "text". > > > The found codec will be textdec of type 'subtitle' even though the stream > > type > > > is attachment. Without the special condition for attachment streams, this > > > is now causing ffprobe to error out with non-zero exit code and incomplete > > > output. > > > > > > > > > > > > Example: > > > > > > [...] > > > Stream #0:9: Attachment: text > > > Metadata: > > > filename: textfile.text > > > mimetype: text/plain > > > [text @ 01AC32310340] Codec type or id mismatches > > > Could not open codec for input stream 9 > > > > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. > > You make a behavioral change to an API function that had this behavior > established and constant over more than 10 years, and when that change > breaks functionality, it's the callers' fault? > How does this go together with all that peanut counting of major, minor > and micro version numbers per library? What is this versioning good for, > when you can make breaking changes and declare the breakage as bugs? We maintain compatibility for valid API usage. We do not maintain bug compatibility. I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT is valid API usage. What do you expect it to do? There are no AVMEDIA_TYPE_ATTACHMENT decoders. More generally, arguments along the line of "change is needed to keep program working>" on their own sound very shady to me and suggest that perhaps program should not be doing whatever it is doing. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
On Sun, Jun 5, 2022 at 10:20 AM Anton Khirnov wrote: > Quoting Soft Works (2022-06-05 09:54:51) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > Anton > > > Khirnov > > > Sent: Sunday, June 5, 2022 9:01 AM > > > To: FFmpeg development discussions and patches < > ffmpeg-devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec > id/type > > > validity checking > > > > > > Quoting Soft Works (2022-06-05 07:23:18) > > > > This is causing a regression in ffprobe. > > > > > > > > The commit removes the special-case check for > AVMEDIA_TYPE_ATTACHMENT which > > > > was required for ffprobe and had been added with > > > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > > > > > The demand from the commit message is not yet guaranteed to be > fulfilled: > > > > > > > > > On entry to avcodec_open2(), the type and id either have to be > > > > > UNKNOWN/NONE or have to match the codec to be used. > > > > > > > > I have one verified example (maybe a second will follow), which is > an MKV > > > with > > > > an attachment "stream" of type "text". > > > > The found codec will be textdec of type 'subtitle' even though the > stream > > > type > > > > is attachment. Without the special condition for attachment streams, > this > > > > is now causing ffprobe to error out with non-zero exit code and > incomplete > > > > output. > > > > > > > > > > > > > > > > > Example: > > > > > > > > [...] > > > > Stream #0:9: Attachment: text > > > > Metadata: > > > > filename: textfile.text > > > > mimetype: text/plain > > > > [text @ 01AC32310340] Codec type or id mismatches > > > > Could not open codec for input stream 9 > > > > > > > > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. > > > > You make a behavioral change to an API function that had this behavior > > established and constant over more than 10 years, and when that change > > breaks functionality, it's the callers' fault? > > How does this go together with all that peanut counting of major, minor > > and micro version numbers per library? What is this versioning good for, > > when you can make breaking changes and declare the breakage as bugs? > > We maintain compatibility for valid API usage. We do not maintain bug > compatibility. > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > is valid API usage. What do you expect it to do? There are no > AVMEDIA_TYPE_ATTACHMENT decoders. > > More generally, arguments along the line of "change is needed to > keep program working>" on their own sound very shady to me and > suggest that perhaps program should not be doing whatever it is > doing. > You broke existing functionality, fix it ASAP or revert those changes! > -- > Anton Khirnov > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 10:20 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > Quoting Soft Works (2022-06-05 09:54:51) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Anton > > > Khirnov > > > Sent: Sunday, June 5, 2022 9:01 AM > > > To: FFmpeg development discussions and patches > > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec > id/type > > > validity checking > > > > > > Quoting Soft Works (2022-06-05 07:23:18) > > > > This is causing a regression in ffprobe. > > > > > > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT > which > > > > was required for ffprobe and had been added with > > > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > > > > > The demand from the commit message is not yet guaranteed to be > fulfilled: > > > > > > > > > On entry to avcodec_open2(), the type and id either have to be > > > > > UNKNOWN/NONE or have to match the codec to be used. > > > > > > > > I have one verified example (maybe a second will follow), which is an > MKV > > > with > > > > an attachment "stream" of type "text". > > > > The found codec will be textdec of type 'subtitle' even though the > stream > > > type > > > > is attachment. Without the special condition for attachment streams, > this > > > > is now causing ffprobe to error out with non-zero exit code and > incomplete > > > > output. > > > > > > > > > > > > --- > - > > > > Example: > > > > > > > > [...] > > > > Stream #0:9: Attachment: text > > > > Metadata: > > > > filename: textfile.text > > > > mimetype: text/plain > > > > [text @ 01AC32310340] Codec type or id mismatches > > > > Could not open codec for input stream 9 > > > > --- > - > > > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. > > > > You make a behavioral change to an API function that had this behavior > > established and constant over more than 10 years, and when that change > > breaks functionality, it's the callers' fault? > > How does this go together with all that peanut counting of major, minor > > and micro version numbers per library? What is this versioning good for, > > when you can make breaking changes and declare the breakage as bugs? > > We maintain compatibility for valid API usage. We do not maintain bug > compatibility. > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > is valid API usage. What do you expect it to do? There are no > AVMEDIA_TYPE_ATTACHMENT decoders. > > More generally, arguments along the line of "change is needed to > keep program working>" on their own sound very shady to me and > suggest that perhaps program should not be doing whatever it is > doing. I might agree to that when: - the function documentation would have been clear about it - it wouldn't be ffprobe code getting invalidated by the change When looking at all the APIs that you are so carefully protecting with those version numbers, there is a small number of APIs, with good to great documentation, but a large number where you can't get the slightest clue about how it is supposed to be used and called and which conditions need to be met, which prerequisites are required before calling, how to interpret and what to do with the results, etc. ...without looking at the ffmpeg/ffprobe code for how these tools are using the APIs. I mean - almost everybody does that. And when somebody is looking at the code of ffprobe to understand how to use the ffmpeg API, then one needs to be able to rely on the code he sees there being correct. And even if it isn't from an ffmpeg internal perspective, I would still consider it as an API break when a change would cause such code fail. sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 10:20 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > Quoting Soft Works (2022-06-05 09:54:51) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Anton > > > Khirnov > > > Sent: Sunday, June 5, 2022 9:01 AM > > > To: FFmpeg development discussions and patches > > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec > id/type > > > validity checking > > > > > > Quoting Soft Works (2022-06-05 07:23:18) > > > > This is causing a regression in ffprobe. > > > > > > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT > which > > > > was required for ffprobe and had been added with > > > e83c716e16c52fa56a78274408f7628e5dc719da. > > > > > > > > The demand from the commit message is not yet guaranteed to be > fulfilled: > > > > > > > > > On entry to avcodec_open2(), the type and id either have to be > > > > > UNKNOWN/NONE or have to match the codec to be used. > > > > > > > > I have one verified example (maybe a second will follow), which is an > MKV > > > with > > > > an attachment "stream" of type "text". > > > > The found codec will be textdec of type 'subtitle' even though the > stream > > > type > > > > is attachment. Without the special condition for attachment streams, > this > > > > is now causing ffprobe to error out with non-zero exit code and > incomplete > > > > output. > > > > > > > > > > > > --- > - > > > > Example: > > > > > > > > [...] > > > > Stream #0:9: Attachment: text > > > > Metadata: > > > > filename: textfile.text > > > > mimetype: text/plain > > > > [text @ 01AC32310340] Codec type or id mismatches > > > > Could not open codec for input stream 9 > > > > --- > - > > > > > > This sounds very much like a bug in ffprobe. It makes no sense to call > > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type. > > > > You make a behavioral change to an API function that had this behavior > > established and constant over more than 10 years, and when that change > > breaks functionality, it's the callers' fault? > > How does this go together with all that peanut counting of major, minor > > and micro version numbers per library? What is this versioning good for, > > when you can make breaking changes and declare the breakage as bugs? > > We maintain compatibility for valid API usage. We do not maintain bug > compatibility. > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > is valid API usage. What do you expect it to do? There are no > AVMEDIA_TYPE_ATTACHMENT decoders. > > More generally, arguments along the line of "change is needed to > keep program working>" on their own sound very shady to me and > suggest that perhaps program should not be doing whatever it is > doing. Shady? I don't understand how you can say that. First of all: You broke ffprobe. It took me one and a half working days to trace this back from the symptom that the user was experiencing to that commit of yours. Now, you could fix that in ffprobe - but then it would be still broken for: 1. Applications which have copied the code from ffprobe 2. Users running (an unchanged) ffprobe with a newer libavcodec version; (you said it would be so important that the libraries are exchangeable and multiple versions can be mixed) Adjusting ffprobe would still break these two cases. Thanks, sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
So much text, but no actual answer. Again: > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > is valid API usage. What do you expect it to do? There are no > AVMEDIA_TYPE_ATTACHMENT decoders. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 12:42 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > So much text, but no actual answer. Again: > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > > is valid API usage. What do you expect it to do? There are no > > AVMEDIA_TYPE_ATTACHMENT decoders. Yes. That's perfectly right. But it doesn't matter at this point. And I used "so much text" to precisely explain why it doesn't matter. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 12:42 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > So much text, but no actual answer. Again: > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > > is valid API usage. What do you expect it to do? There are no > > AVMEDIA_TYPE_ATTACHMENT decoders. As you didn't mention anything about how you want to address it, does it mean that your intention is to leave it as is and declare all other code being wrong? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c to remove MAX_PATH limit.
---
libavutil/wchar_filename.h | 51 ++
1 file changed, 51 insertions(+)
diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f0824..c0e5d47 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,57 @@ static inline int utf8towchar(const char *filename_utf8,
wchar_t **filename_w)
MultiByteToWideChar(CP_UTF8, 0, filename_utf8, -1, *filename_w, num_chars);
return 0;
}
+
+av_warn_unused_result
+static inline int wchartocp(unsigned int code_page, const wchar_t *filename_w,
+char **filename)
+{
+DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
+NULL, 0, NULL, NULL);
+if (num_chars <= 0) {
+*filename = NULL;
+return 0;
+}
+*filename = av_calloc(num_chars, sizeof(char));
+if (!*filename) {
+errno = ENOMEM;
+return -1;
+}
+WideCharToMultiByte(code_page, flags, filename_w, -1,
+*filename, num_chars, NULL, NULL);
+return 0;
+}
+
+av_warn_unused_result
+static inline int wchartoutf8(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_UTF8, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int wchartoansi(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_ACP, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int utf8toansi(const char *filename_utf8, char **filename)
+{
+wchar_t *filename_w = NULL;
+int ret = -1;
+if (utf8towchar(filename_utf8, &filename_w))
+return -1;
+
+if (!filename_w) {
+*filename = NULL;
+return 0;
+}
+
+ret = wchartoansi(filename_w, filename);
+av_free(filename_w);
+return ret;
+}
#endif
#endif /* AVUTIL_WCHAR_FILENAME_H */
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c to remove MAX_PATH limit.
---
libavutil/wchar_filename.h | 51 ++
1 file changed, 51 insertions(+)
diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f0824..c0e5d47 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,57 @@ static inline int utf8towchar(const char *filename_utf8,
wchar_t **filename_w)
MultiByteToWideChar(CP_UTF8, 0, filename_utf8, -1, *filename_w, num_chars);
return 0;
}
+
+av_warn_unused_result
+static inline int wchartocp(unsigned int code_page, const wchar_t *filename_w,
+char **filename)
+{
+DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
+NULL, 0, NULL, NULL);
+if (num_chars <= 0) {
+*filename = NULL;
+return 0;
+}
+*filename = av_calloc(num_chars, sizeof(char));
+if (!*filename) {
+errno = ENOMEM;
+return -1;
+}
+WideCharToMultiByte(code_page, flags, filename_w, -1,
+*filename, num_chars, NULL, NULL);
+return 0;
+}
+
+av_warn_unused_result
+static inline int wchartoutf8(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_UTF8, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int wchartoansi(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_ACP, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int utf8toansi(const char *filename_utf8, char **filename)
+{
+wchar_t *filename_w = NULL;
+int ret = -1;
+if (utf8towchar(filename_utf8, &filename_w))
+return -1;
+
+if (!filename_w) {
+*filename = NULL;
+return 0;
+}
+
+ret = wchartoansi(filename_w, filename);
+av_free(filename_w);
+return ret;
+}
#endif
#endif /* AVUTIL_WCHAR_FILENAME_H */
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 3/4] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
---
compat/w32dlfcn.h | 80 +--
1 file changed, 64 insertions(+), 16 deletions(-)
diff --git a/compat/w32dlfcn.h b/compat/w32dlfcn.h
index 52a94ef..6b0dd7d 100644
--- a/compat/w32dlfcn.h
+++ b/compat/w32dlfcn.h
@@ -22,9 +22,31 @@
#ifdef _WIN32
#include
#include "config.h"
-#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT
#include "libavutil/wchar_filename.h"
-#endif
+
+static inline wchar_t *get_module_filename(HMODULE module)
+{
+wchar_t *path = NULL, *new_path = NULL;
+DWORD path_size = 0, path_len = 0;
+
+do {
+path_size = path_size ? 2 * path_size : MAX_PATH;
+new_path = av_realloc_array(path, path_size, sizeof *path);
+if (!new_path) {
+av_free(path);
+return NULL;
+}
+path = new_path;
+path_len = GetModuleFileNameW(module, path, path_size);
+} while (path_len && path_size <= 32768 && path_size <= path_len);
+
+if (!path_len) {
+av_free(path);
+return NULL;
+}
+return path;
+}
+
/**
* Safe function used to open dynamic libs. This attempts to improve program
security
* by removing the current directory from the dll search path. Only dll's
found in the
@@ -34,29 +56,53 @@
*/
static inline HMODULE win32_dlopen(const char *name)
{
+wchar_t *name_w = NULL;
+if (utf8towchar(name, &name_w))
+name_w = NULL;
#if _WIN32_WINNT < 0x0602
// Need to check if KB2533623 is available
if (!GetProcAddress(GetModuleHandleW(L"kernel32.dll"),
"SetDefaultDllDirectories")) {
HMODULE module = NULL;
-wchar_t *path = NULL, *name_w = NULL;
-DWORD pathlen;
-if (utf8towchar(name, &name_w))
+wchar_t *path = NULL, *new_path = NULL;
+DWORD pathlen, pathsize, namelen;
+if (!name_w)
goto exit;
-path = (wchar_t *)av_calloc(MAX_PATH, sizeof(wchar_t));
+namelen = wcslen(name_w);
// Try local directory first
-pathlen = GetModuleFileNameW(NULL, path, MAX_PATH);
-pathlen = wcsrchr(path, '\\') - path;
-if (pathlen == 0 || pathlen + wcslen(name_w) + 2 > MAX_PATH)
+path = get_module_filename(NULL);
+if (!path)
goto exit;
-path[pathlen] = '\\';
+new_path = wcsrchr(path, '\\');
+if (!new_path)
+goto exit;
+pathlen = new_path - path;
+pathsize = pathlen + namelen + 2;
+new_path = av_realloc_array(path, pathsize, sizeof *path);
+if (!new_path)
+goto exit;
+path = new_path;
wcscpy(path + pathlen + 1, name_w);
module = LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
if (module == NULL) {
// Next try System32 directory
-pathlen = GetSystemDirectoryW(path, MAX_PATH);
-if (pathlen == 0 || pathlen + wcslen(name_w) + 2 > MAX_PATH)
+pathlen = GetSystemDirectoryW(path, pathsize);
+if (!pathlen)
goto exit;
-path[pathlen] = '\\';
+// Buffer is not enough in two cases:
+// 1. system directory + \ + module name
+// 2. system directory even without module name.
+if (pathlen + namelen + 2 > pathsize) {
+pathsize = pathlen + namelen + 2;
+new_path = av_realloc_array(path, pathsize, sizeof *path);
+if (!new_path)
+goto exit;
+path = new_path;
+// Query again to handle case #2.
+pathlen = GetSystemDirectoryW(path, pathsize);
+if (!pathlen)
+goto exit;
+}
+path[pathlen] = L'\\';
wcscpy(path + pathlen + 1, name_w);
module = LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
}
@@ -73,15 +119,17 @@ exit:
# define LOAD_LIBRARY_SEARCH_SYSTEM320x0800
#endif
#if HAVE_WINRT
-wchar_t *name_w = NULL;
int ret;
-if (utf8towchar(name, &name_w))
+if (!name_w)
return NULL;
ret = LoadPackagedLibrary(name_w, 0);
av_free(name_w);
return ret;
#else
-return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR |
LOAD_LIBRARY_SEARCH_SYSTEM32);
+/* filename may be be in CP_ACP */
+if (!name_w)
+return LoadLibraryExA(name, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR
| LOAD_LIBRARY_SEARCH_SYSTEM32);
+return LoadLibraryExW(name_w, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR |
LOAD_LIBRARY_SEARCH_SYSTEM32);
#endif
}
#define dlopen(name, flags) win32_dlopen(name)
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 2/4] libavformat/avisynth.c: Remove MAX_PATH limit
---
libavformat/avisynth.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 8ba2bde..f7bea8c 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -34,6 +34,7 @@
/* Platform-specific directives. */
#ifdef _WIN32
#include "compat/w32dlfcn.h"
+ #include "libavutil/wchar_filename.h"
#undef EXTERN_C
#define AVISYNTH_LIB "avisynth"
#else
@@ -810,8 +811,7 @@ static int avisynth_open_file(AVFormatContext *s)
AVS_Value arg, val;
int ret;
#ifdef _WIN32
-char filename_ansi[MAX_PATH * 4];
-wchar_t filename_wc[MAX_PATH * 4];
+char *filename_ansi = NULL;
#endif
if (ret = avisynth_context_create(s))
@@ -819,10 +819,12 @@ static int avisynth_open_file(AVFormatContext *s)
#ifdef _WIN32
/* Convert UTF-8 to ANSI code page */
-MultiByteToWideChar(CP_UTF8, 0, s->url, -1, filename_wc, MAX_PATH * 4);
-WideCharToMultiByte(CP_THREAD_ACP, 0, filename_wc, -1, filename_ansi,
-MAX_PATH * 4, NULL, NULL);
+if (utf8toansi(s->url, &filename_ansi)) {
+ret = AVERROR_UNKNOWN;
+goto fail;
+}
arg = avs_new_value_string(filename_ansi);
+av_free(filename_ansi);
#else
arg = avs_new_value_string(s->url);
#endif
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 4/4] fftools/cmdutils.c: Remove MAX_PATH limit
---
fftools/cmdutils.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 5d7cdc3..d42bb04 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -50,6 +50,7 @@
#include "opt_common.h"
#ifdef _WIN32
#include
+#include "compat/w32dlfcn.h"
#endif
AVDictionary *sws_dict;
@@ -812,6 +813,9 @@ FILE *get_preset_file(char *filename, size_t filename_size,
{
FILE *f = NULL;
int i;
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+char *datadir = NULL;
+#endif
const char *base[3] = { getenv("FFMPEG_DATADIR"),
getenv("HOME"),
FFMPEG_DATADIR, };
@@ -821,19 +825,31 @@ FILE *get_preset_file(char *filename, size_t
filename_size,
f = fopen(filename, "r");
} else {
#if HAVE_GETMODULEHANDLE && defined(_WIN32)
-char datadir[MAX_PATH], *ls;
+wchar_t *datadir_w = get_module_filename(NULL);
base[2] = NULL;
-if (GetModuleFileNameA(GetModuleHandleA(NULL), datadir,
sizeof(datadir) - 1))
+if (wchartoansi(datadir_w, &datadir))
+datadir = NULL;
+av_free(datadir_w);
+
+if (datadir)
{
-for (ls = datadir; ls < datadir + strlen(datadir); ls++)
+char *ls;
+for (ls = datadir; *ls; ls++)
if (*ls == '\\') *ls = '/';
if (ls = strrchr(datadir, '/'))
{
-*ls = 0;
-strncat(datadir, "/ffpresets", sizeof(datadir) - 1 -
strlen(datadir));
-base[2] = datadir;
+ptrdiff_t datadir_len = ls - datadir;
+size_t desired_size = datadir_len + strlen("/ffpresets") + 1;
+char *new_datadir = av_realloc_array(
+datadir, desired_size, sizeof *datadir);
+if (new_datadir) {
+datadir = new_datadir;
+datadir[datadir_len] = 0;
+strncat(datadir, "/ffpresets", desired_size - 1 -
datadir_len);
+base[2] = datadir;
+}
}
}
#endif
@@ -853,6 +869,9 @@ FILE *get_preset_file(char *filename, size_t filename_size,
}
}
+#if HAVE_GETMODULEHANDLE && defined(_WIN32)
+av_free(datadir);
+#endif
return f;
}
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
These functions are going to be used in libavformat/avisynth.c
and fftools/cmdutils.c to remove MAX_PATH limit.
---
libavutil/wchar_filename.h | 51 ++
1 file changed, 51 insertions(+)
diff --git a/libavutil/wchar_filename.h b/libavutil/wchar_filename.h
index 90f0824..c0e5d47 100644
--- a/libavutil/wchar_filename.h
+++ b/libavutil/wchar_filename.h
@@ -40,6 +40,57 @@ static inline int utf8towchar(const char *filename_utf8,
wchar_t **filename_w)
MultiByteToWideChar(CP_UTF8, 0, filename_utf8, -1, *filename_w, num_chars);
return 0;
}
+
+av_warn_unused_result
+static inline int wchartocp(unsigned int code_page, const wchar_t *filename_w,
+char **filename)
+{
+DWORD flags = code_page == CP_UTF8 ? MB_ERR_INVALID_CHARS : 0;
+int num_chars = WideCharToMultiByte(code_page, flags, filename_w, -1,
+NULL, 0, NULL, NULL);
+if (num_chars <= 0) {
+*filename = NULL;
+return 0;
+}
+*filename = av_calloc(num_chars, sizeof(char));
+if (!*filename) {
+errno = ENOMEM;
+return -1;
+}
+WideCharToMultiByte(code_page, flags, filename_w, -1,
+*filename, num_chars, NULL, NULL);
+return 0;
+}
+
+av_warn_unused_result
+static inline int wchartoutf8(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_UTF8, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int wchartoansi(const wchar_t *filename_w, char **filename)
+{
+return wchartocp(CP_ACP, filename_w, filename);
+}
+
+av_warn_unused_result
+static inline int utf8toansi(const char *filename_utf8, char **filename)
+{
+wchar_t *filename_w = NULL;
+int ret = -1;
+if (utf8towchar(filename_utf8, &filename_w))
+return -1;
+
+if (!filename_w) {
+*filename = NULL;
+return 0;
+}
+
+ret = wchartoansi(filename_w, filename);
+av_free(filename_w);
+return ret;
+}
#endif
#endif /* AVUTIL_WCHAR_FILENAME_H */
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 3/6] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW
#if (_WIN32_WINNT < 0x0602) || HAVE_WINRT #include "libavutil/wchar_filename.h" #endif caused build error due to utf8towchar being undefined. Made wchar_filename.h include unconditional. Also removed manifest changes since it was decided to adopt \\?\ prefixes instead. New version is at https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297198.html. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] tools/target_dec_fuzzer: add a custom get_buffer2() implementation
On 5/31/2022 5:42 PM, James Almer wrote:
Unlike avcodec_default_get_buffer2(), this version does not allocate more than
what the normal image helper functions consider should be allocated for a given
frame.
Since the get_buffer2() documentation does not require any kind of buffer
overallocation for any of the planes, this should help detect bugs in our DR1
decoders if they overread beyond the end of the buffer, simulating what some
library users might experience when they use their own custom get_buffer2()
implementations.
Signed-off-by: James Almer
---
Now making sure to not allocate more plane buffers than needed.
tools/target_dec_fuzzer.c | 52 +++
1 file changed, 52 insertions(+)
diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index 288aa63313..2e43ed3d88 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -104,6 +104,57 @@ const uint32_t maxiteration = 8096;
static const uint64_t FUZZ_TAG = 0x4741542D5A5A5546ULL;
+static int fuzz_video_get_buffer(AVCodecContext *ctx, AVFrame *frame)
+{
+ptrdiff_t linesize1[4];
+size_t size[4];
+int linesize_align[AV_NUM_DATA_POINTERS];
+int ret, w = frame->width, h = frame->height;
+
+avcodec_align_dimensions2(ctx, &w, &h, linesize_align);
+ret = av_image_fill_linesizes(frame->linesize, ctx->pix_fmt, w);
+if (ret < 0)
+return ret;
+
+for (int i = 0; i < 4; i++)
+linesize1[i] = frame->linesize[i] =
+FFALIGN(frame->linesize[i], linesize_align[i]);
+
+ret = av_image_fill_plane_sizes(size, ctx->pix_fmt, h, linesize1);
+if (ret < 0)
+goto fail;
+
+for (int i = 0; i < 4; i++) {
+if (!size[i])
+break;
+frame->buf[i] = av_buffer_alloc(size[i]);
+if (!frame->buf[i]) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+frame->data[i] = frame->buf[i]->data;
+}
+
+fail:
+if (ret < 0)
+av_frame_unref(frame);
+return ret;
+}
+
+static int fuzz_get_buffer2(AVCodecContext *ctx, AVFrame *frame, int flags)
+{
+switch (ctx->codec_type) {
+case AVMEDIA_TYPE_VIDEO:
+return (ctx->codec->capabilities & AV_CODEC_CAP_DR1)
+ ? fuzz_video_get_buffer(ctx, frame)
+ : avcodec_default_get_buffer2(ctx, frame, flags);
+case AVMEDIA_TYPE_AUDIO:
+return avcodec_default_get_buffer2(ctx, frame, flags);
+default:
+return AVERROR(EINVAL);
+}
+}
+
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
uint64_t maxpixels_per_frame = 4096 * 4096;
uint64_t maxpixels;
@@ -241,6 +292,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
ctx->max_pixels = maxpixels_per_frame; //To reduce false positive OOM
and hangs
ctx->max_samples = maxsamples_per_frame;
+ctx->get_buffer2 = fuzz_get_buffer2;
if (size > 1024) {
GetByteContext gbc;
Will push soon if nobody objects.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/remove_extradata_bsf: add a list of supported codec ids
On 5/25/2022 7:52 PM, James Almer wrote:
There's no point allowing the use of this filter for codecs where
it will silently do nothing.
Signed-off-by: James Almer
---
libavcodec/remove_extradata_bsf.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/libavcodec/remove_extradata_bsf.c
b/libavcodec/remove_extradata_bsf.c
index 66b7d00bd8..584213e40f 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -18,6 +18,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
*/
+#include "libavutil/avassert.h"
#include "libavutil/log.h"
#include "libavutil/opt.h"
@@ -217,7 +218,7 @@ static int remove_extradata(AVBSFContext *ctx, AVPacket *pkt)
i = vc1_split(pkt->data, pkt->size);
break;
default:
-i = 0;
+av_assert0(0);
}
pkt->data += i;
@@ -238,6 +239,20 @@ static const AVOption options[] = {
{ NULL },
};
+static const enum AVCodecID codec_ids[] = {
+AV_CODEC_ID_AV1,
+AV_CODEC_ID_AVS2,
+AV_CODEC_ID_AVS3,
+AV_CODEC_ID_CAVS,
+AV_CODEC_ID_H264,
+AV_CODEC_ID_HEVC,
+AV_CODEC_ID_MPEG1VIDEO,
+AV_CODEC_ID_MPEG2VIDEO,
+AV_CODEC_ID_MPEG4,
+AV_CODEC_ID_VC1,
+AV_CODEC_ID_NONE,
+};
+
static const AVClass remove_extradata_class = {
.class_name = "remove_extradata",
.item_name = av_default_item_name,
@@ -247,6 +262,7 @@ static const AVClass remove_extradata_class = {
const FFBitStreamFilter ff_remove_extradata_bsf = {
.p.name = "remove_extra",
+.p.codec_ids= codec_ids,
.p.priv_class = &remove_extradata_class,
.priv_data_size = sizeof(RemoveExtradataContext),
.filter = remove_extradata,
Will apply.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
Quoting Soft Works (2022-06-05 13:10:49) > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Anton > > Khirnov > > Sent: Sunday, June 5, 2022 12:42 PM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > > validity checking > > > > So much text, but no actual answer. Again: > > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > > > is valid API usage. What do you expect it to do? There are no > > > AVMEDIA_TYPE_ATTACHMENT decoders. > > As you didn't mention anything about how you want to address it, does it > mean that your intention is to leave it as is and declare all other code > being wrong? Frankly, your attitude of breathlessly repeating "ffprobe is BROKEN, and this is HORRIBLE" is unhelpful. I am open to various kinds of solutions, which include (temporarily?) reintroducing previous behavior, but first we must determine what the actual issue is. I.e. whether it is libavcodec or ffprobe that is broken. You seem uninterested in this question, which makes me not very interested in spending time on this. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
> -Original Message- > From: ffmpeg-devel On Behalf Of Anton > Khirnov > Sent: Sunday, June 5, 2022 3:21 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type > validity checking > > Quoting Soft Works (2022-06-05 13:10:49) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Anton > > > Khirnov > > > Sent: Sunday, June 5, 2022 12:42 PM > > > To: FFmpeg development discussions and patches > > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec > id/type > > > validity checking > > > > > > So much text, but no actual answer. Again: > > > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT > > > > is valid API usage. What do you expect it to do? There are no > > > > AVMEDIA_TYPE_ATTACHMENT decoders. > > > > As you didn't mention anything about how you want to address it, does it > > mean that your intention is to leave it as is and declare all other code > > being wrong? > > Frankly, your attitude of breathlessly repeating "ffprobe is BROKEN, and > this is HORRIBLE" is unhelpful. > I am open to various kinds of solutions, which include (temporarily?) > reintroducing previous behavior, but first we must determine what the > actual issue is. I.e. whether it is libavcodec or ffprobe that is > broken. You seem uninterested in this question, which makes me not very > interested in spending time on this. I need to fight about every single character of submitted code, and you are trying to justify your commit that clearly breaks behavior instead of either reverting or offering a solution. Instead I need to go through stupid discussions with you. I don't understand that behavior. For most others it would be totally clear that such commit would need to be reverted until a new solution is found. I have reported the issue nicely and well explained. But you start to find some justifications instead of suggesting any solution. I don't like that. I wish I wouldn't have been required to write that much text, and you would have just responded something like, OK, I'll see how I can resolve the regression that my commit has caused. That would be a normal reaction IMO. Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avutil/frame: add av_frame_replace
Signed-off-by: James Almer
---
Now taking into account the new channel layout API fields, supporting non
refcouted src, plus changing the documentation to state that the dst frame
properties will be replaced by those from src even if the data described
by both frames is the same.
Still missing APIChanges entry and version bump.
libavutil/frame.c | 144 ++
libavutil/frame.h | 17 ++
2 files changed, 161 insertions(+)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 4c16488c66..2a4a2c27ad 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -461,6 +461,150 @@ fail:
return ret;
}
+int av_frame_replace(AVFrame *dst, const AVFrame *src)
+{
+int i, ret = 0;
+
+dst->format = src->format;
+dst->width = src->width;
+dst->height = src->height;
+dst->nb_samples = src->nb_samples;
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+dst->channels = src->channels;
+dst->channel_layout = src->channel_layout;
+if (!av_channel_layout_check(&src->ch_layout)) {
+av_channel_layout_uninit(&dst->ch_layout);
+if (src->channel_layout)
+av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout);
+else {
+dst->ch_layout.nb_channels = src->channels;
+dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
+}
+} else {
+#endif
+ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
+if (ret < 0)
+goto fail;
+#if FF_API_OLD_CHANNEL_LAYOUT
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+wipe_side_data(dst);
+av_dict_free(&dst->metadata);
+ret = frame_copy_props(dst, src, 0);
+if (ret < 0)
+goto fail;
+
+/* duplicate the frame data if it's not refcounted */
+if (!src->buf[0]) {
+for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++)
+av_buffer_unref(&dst->buf[i]);
+for (i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+
+memset(dst->data, 0, sizeof(dst->data));
+if (dst->extended_data != dst->data)
+av_freep(&dst->extended_data);
+
+ret = av_frame_get_buffer(dst, 0);
+if (ret < 0)
+goto fail;
+
+ret = av_frame_copy(dst, src);
+if (ret < 0)
+goto fail;
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+return 0;
+}
+
+/* replace the buffers */
+for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) {
+ret = av_buffer_replace(&dst->buf[i], src->buf[i]);
+if (ret < 0)
+goto fail;
+}
+
+if (src->extended_buf) {
+if (dst->nb_extended_buf != src->nb_extended_buf) {
+int nb_extended_buf = FFMIN(dst->nb_extended_buf,
src->nb_extended_buf);
+void *tmp;
+
+for (i = nb_extended_buf; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+
+tmp = av_realloc_array(dst->extended_buf,
sizeof(*dst->extended_buf),
+ src->nb_extended_buf);
+if (!tmp) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+dst->extended_buf = tmp;
+dst->nb_extended_buf = src->nb_extended_buf;
+
+memset(&dst->extended_buf[nb_extended_buf], 0,
+ (src->nb_extended_buf - nb_extended_buf) *
sizeof(*dst->extended_buf));
+}
+
+for (i = 0; i < src->nb_extended_buf; i++) {
+ret = av_buffer_replace(&dst->extended_buf[i],
src->extended_buf[i]);
+if (ret < 0)
+goto fail;
+}
+} else if (dst->extended_buf) {
+for (i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+}
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+if (dst->extended_data != dst->data)
+av_freep(&dst->extended_data);
+
+if (src->extended_data != src->data) {
+int ch = src->ch_layout.nb_channels;
+
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+if (!ch) {
+ch = src->channels;
+CHECK_CHANNELS_CONSISTENCY(src);
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+if (!ch) {
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch);
+if (!dst->extended_data) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(dst->extended_data, src->extended_data,
sizeof(*src->extended_data) * ch);
+} else
+dst->extended_data = dst->data;
+
+memcpy(dst->data, src->data, siz
Re: [FFmpeg-devel] [PATCH] avutil/frame: add av_frame_replace
James Almer:
> Signed-off-by: James Almer
> ---
> Now taking into account the new channel layout API fields, supporting non
> refcouted src, plus changing the documentation to state that the dst frame
> properties will be replaced by those from src even if the data described
> by both frames is the same.
>
Apparently, you didn't get my point last time: What happens if dst and
src coincide did not mean "what happpens if the data both frames refer
to already coincides"; it meant "what happens if src == dst?" (so that
there is only one AVFrame involved). You either disallow this altogether
or you support it. This patch (and the last one) does neither.
> Still missing APIChanges entry and version bump.
>
> libavutil/frame.c | 144 ++
> libavutil/frame.h | 17 ++
> 2 files changed, 161 insertions(+)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 4c16488c66..2a4a2c27ad 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -461,6 +461,150 @@ fail:
> return ret;
> }
>
> +int av_frame_replace(AVFrame *dst, const AVFrame *src)
> +{
> +int i, ret = 0;
Why don't you use "for (int i = 0;" loop variables?
> +
> +dst->format = src->format;
> +dst->width = src->width;
> +dst->height = src->height;
> +dst->nb_samples = src->nb_samples;
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +dst->channels = src->channels;
> +dst->channel_layout = src->channel_layout;
> +if (!av_channel_layout_check(&src->ch_layout)) {
> +av_channel_layout_uninit(&dst->ch_layout);
> +if (src->channel_layout)
> +av_channel_layout_from_mask(&dst->ch_layout,
> src->channel_layout);
> +else {
> +dst->ch_layout.nb_channels = src->channels;
> +dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
> +}
> +} else {
> +#endif
> +ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
> +if (ret < 0)
> +goto fail;
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +}
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> +wipe_side_data(dst);
> +av_dict_free(&dst->metadata);
> +ret = frame_copy_props(dst, src, 0);
> +if (ret < 0)
> +goto fail;
> +
> +/* duplicate the frame data if it's not refcounted */
> +if (!src->buf[0]) {
> +for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++)
> +av_buffer_unref(&dst->buf[i]);
> +for (i = 0; i < dst->nb_extended_buf; i++)
> +av_buffer_unref(&dst->extended_buf[i]);
> +av_freep(&dst->extended_buf);
> +
> +memset(dst->data, 0, sizeof(dst->data));
> +if (dst->extended_data != dst->data)
> +av_freep(&dst->extended_data);
> +
> +ret = av_frame_get_buffer(dst, 0);
> +if (ret < 0)
> +goto fail;
> +
> +ret = av_frame_copy(dst, src);
> +if (ret < 0)
> +goto fail;
> +
> +ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
> +if (ret < 0)
> +goto fail;
> +
> +return 0;
> +}
> +
> +/* replace the buffers */
> +for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) {
> +ret = av_buffer_replace(&dst->buf[i], src->buf[i]);
> +if (ret < 0)
> +goto fail;
> +}
> +
> +if (src->extended_buf) {
> +if (dst->nb_extended_buf != src->nb_extended_buf) {
> +int nb_extended_buf = FFMIN(dst->nb_extended_buf,
> src->nb_extended_buf);
> +void *tmp;
> +
> +for (i = nb_extended_buf; i < dst->nb_extended_buf; i++)
> +av_buffer_unref(&dst->extended_buf[i]);
> +
> +tmp = av_realloc_array(dst->extended_buf,
> sizeof(*dst->extended_buf),
> + src->nb_extended_buf);
> +if (!tmp) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
> +dst->extended_buf = tmp;
> +dst->nb_extended_buf = src->nb_extended_buf;
> +
> +memset(&dst->extended_buf[nb_extended_buf], 0,
> + (src->nb_extended_buf - nb_extended_buf) *
> sizeof(*dst->extended_buf));
> +}
> +
> +for (i = 0; i < src->nb_extended_buf; i++) {
> +ret = av_buffer_replace(&dst->extended_buf[i],
> src->extended_buf[i]);
> +if (ret < 0)
> +goto fail;
> +}
> +} else if (dst->extended_buf) {
> +for (i = 0; i < dst->nb_extended_buf; i++)
> +av_buffer_unref(&dst->extended_buf[i]);
> +av_freep(&dst->extended_buf);
> +}
> +
> +ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
> +if (ret < 0)
> +goto fail;
> +
> +if (dst->extended_data != dst->data)
> +av_freep(&dst->extended_data);
> +
> +if (src->extended_data != src->data) {
> +int ch = src->ch_layout.nb_chann
Re: [FFmpeg-devel] [PATCH] avutil/frame: add av_frame_replace
On 6/5/2022 12:42 PM, Andreas Rheinhardt wrote:
James Almer:
Signed-off-by: James Almer
---
Now taking into account the new channel layout API fields, supporting non
refcouted src, plus changing the documentation to state that the dst frame
properties will be replaced by those from src even if the data described
by both frames is the same.
Apparently, you didn't get my point last time: What happens if dst and
src coincide did not mean "what happpens if the data both frames refer
to already coincides"; it meant "what happens if src == dst?" (so that
there is only one AVFrame involved). You either disallow this altogether
or you support it. This patch (and the last one) does neither.
Ok, will disallow this with
if (dst == src)
return AVERROR(EINVAL);
How does av_frame_ref() handle this scenario, for that matter? The
checks i see to ensure dst is clean (and thus one could assume is
different than src) are non strict with av_assert1().
Still missing APIChanges entry and version bump.
libavutil/frame.c | 144 ++
libavutil/frame.h | 17 ++
2 files changed, 161 insertions(+)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 4c16488c66..2a4a2c27ad 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -461,6 +461,150 @@ fail:
return ret;
}
+int av_frame_replace(AVFrame *dst, const AVFrame *src)
+{
+int i, ret = 0;
Why don't you use "for (int i = 0;" loop variables?
Because i copied this from av_frame_ref(). Will change it.
+
+dst->format = src->format;
+dst->width = src->width;
+dst->height = src->height;
+dst->nb_samples = src->nb_samples;
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+dst->channels = src->channels;
+dst->channel_layout = src->channel_layout;
+if (!av_channel_layout_check(&src->ch_layout)) {
+av_channel_layout_uninit(&dst->ch_layout);
+if (src->channel_layout)
+av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout);
+else {
+dst->ch_layout.nb_channels = src->channels;
+dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
+}
+} else {
+#endif
+ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
+if (ret < 0)
+goto fail;
+#if FF_API_OLD_CHANNEL_LAYOUT
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+wipe_side_data(dst);
+av_dict_free(&dst->metadata);
+ret = frame_copy_props(dst, src, 0);
+if (ret < 0)
+goto fail;
+
+/* duplicate the frame data if it's not refcounted */
+if (!src->buf[0]) {
+for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++)
+av_buffer_unref(&dst->buf[i]);
+for (i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+
+memset(dst->data, 0, sizeof(dst->data));
+if (dst->extended_data != dst->data)
+av_freep(&dst->extended_data);
+
+ret = av_frame_get_buffer(dst, 0);
+if (ret < 0)
+goto fail;
+
+ret = av_frame_copy(dst, src);
+if (ret < 0)
+goto fail;
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+return 0;
+}
+
+/* replace the buffers */
+for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) {
+ret = av_buffer_replace(&dst->buf[i], src->buf[i]);
+if (ret < 0)
+goto fail;
+}
+
+if (src->extended_buf) {
+if (dst->nb_extended_buf != src->nb_extended_buf) {
+int nb_extended_buf = FFMIN(dst->nb_extended_buf,
src->nb_extended_buf);
+void *tmp;
+
+for (i = nb_extended_buf; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+
+tmp = av_realloc_array(dst->extended_buf,
sizeof(*dst->extended_buf),
+ src->nb_extended_buf);
+if (!tmp) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+dst->extended_buf = tmp;
+dst->nb_extended_buf = src->nb_extended_buf;
+
+memset(&dst->extended_buf[nb_extended_buf], 0,
+ (src->nb_extended_buf - nb_extended_buf) *
sizeof(*dst->extended_buf));
+}
+
+for (i = 0; i < src->nb_extended_buf; i++) {
+ret = av_buffer_replace(&dst->extended_buf[i],
src->extended_buf[i]);
+if (ret < 0)
+goto fail;
+}
+} else if (dst->extended_buf) {
+for (i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+}
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+if (dst->extended_data != dst->data)
+
[FFmpeg-devel] [PATCH v2] avutil/frame: add av_frame_replace
Signed-off-by: James Almer
---
Now disallowing src == dst.
libavutil/frame.c | 147 ++
libavutil/frame.h | 13
2 files changed, 160 insertions(+)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 4c16488c66..86f9dcb59a 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -461,6 +461,153 @@ fail:
return ret;
}
+int av_frame_replace(AVFrame *dst, const AVFrame *src)
+{
+int ret = 0;
+
+if (dst == src)
+return AVERROR(EINVAL);
+
+dst->format = src->format;
+dst->width = src->width;
+dst->height = src->height;
+dst->nb_samples = src->nb_samples;
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+dst->channels = src->channels;
+dst->channel_layout = src->channel_layout;
+if (!av_channel_layout_check(&src->ch_layout)) {
+av_channel_layout_uninit(&dst->ch_layout);
+if (src->channel_layout)
+av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout);
+else {
+dst->ch_layout.nb_channels = src->channels;
+dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
+}
+} else {
+#endif
+ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout);
+if (ret < 0)
+goto fail;
+#if FF_API_OLD_CHANNEL_LAYOUT
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+wipe_side_data(dst);
+av_dict_free(&dst->metadata);
+ret = frame_copy_props(dst, src, 0);
+if (ret < 0)
+goto fail;
+
+/* duplicate the frame data if it's not refcounted */
+if (!src->buf[0]) {
+for (int i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++)
+av_buffer_unref(&dst->buf[i]);
+for (int i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+
+memset(dst->data, 0, sizeof(dst->data));
+if (dst->extended_data != dst->data)
+av_freep(&dst->extended_data);
+
+ret = av_frame_get_buffer(dst, 0);
+if (ret < 0)
+goto fail;
+
+ret = av_frame_copy(dst, src);
+if (ret < 0)
+goto fail;
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+return 0;
+}
+
+/* replace the buffers */
+for (int i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) {
+ret = av_buffer_replace(&dst->buf[i], src->buf[i]);
+if (ret < 0)
+goto fail;
+}
+
+if (src->extended_buf) {
+if (dst->nb_extended_buf != src->nb_extended_buf) {
+int nb_extended_buf = FFMIN(dst->nb_extended_buf,
src->nb_extended_buf);
+void *tmp;
+
+for (int i = nb_extended_buf; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+
+tmp = av_realloc_array(dst->extended_buf,
sizeof(*dst->extended_buf),
+ src->nb_extended_buf);
+if (!tmp) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+dst->extended_buf = tmp;
+dst->nb_extended_buf = src->nb_extended_buf;
+
+memset(&dst->extended_buf[nb_extended_buf], 0,
+ (src->nb_extended_buf - nb_extended_buf) *
sizeof(*dst->extended_buf));
+}
+
+for (int i = 0; i < src->nb_extended_buf; i++) {
+ret = av_buffer_replace(&dst->extended_buf[i],
src->extended_buf[i]);
+if (ret < 0)
+goto fail;
+}
+} else if (dst->extended_buf) {
+for (int i = 0; i < dst->nb_extended_buf; i++)
+av_buffer_unref(&dst->extended_buf[i]);
+av_freep(&dst->extended_buf);
+}
+
+ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx);
+if (ret < 0)
+goto fail;
+
+if (dst->extended_data != dst->data)
+av_freep(&dst->extended_data);
+
+if (src->extended_data != src->data) {
+int ch = src->ch_layout.nb_channels;
+
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+if (!ch) {
+ch = src->channels;
+CHECK_CHANNELS_CONSISTENCY(src);
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+if (!ch) {
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch);
+if (!dst->extended_data) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(dst->extended_data, src->extended_data,
sizeof(*src->extended_data) * ch);
+} else
+dst->extended_data = dst->data;
+
+memcpy(dst->data, src->data, sizeof(src->data));
+memcpy(dst->linesize, src->linesize, sizeof(src->linesize));
+
+return 0;
+
+fail:
+av_frame_unref(dst);
+return ret;
+}
+
AVFrame *av_frame_clone(const AVFrame *
[FFmpeg-devel] Collective Ping
Hello, I’d like to kindly PING about the following submissions. libavformat/asf: fix handling of byte array length values https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=6653 => No pending questions or objections Add derive-device function which searches for existing devices in both directions https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=6655 => pending review from Mark Thompson Support long file names on Windows https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=6701 => No pending questions or objections libx264: Set min build version to 158 https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.30.v6.ffstaging.ffmpeg.1653568143191.ffmpegag...@gmail.com/ => No pending questions or objections avcodec/dvdsubdec, dvbsubdec: remove bitmap dumping in DEBUG builds https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.17.v3.ffstaging.ffmpeg.1653749538475.ffmpegag...@gmail.com/ => No pending questions or objections libavutil/tests/md5: Remove 'volatile workaround' to avoid warnings https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.20.v4.ffstaging.ffmpeg.1653750420780.ffmpegag...@gmail.com/ => No pending questions or objections Please let me know whether there are any questions or concerns left with any of the above. Thanks, sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3] libavcodec/qsvenc: add ROI support to qsv encoder
Use The mfxEncoderCtrl parameter to enable ROI. Get side data
"AVRegionOfInterest" and use it to configure "mfxExtEncoderROI" which is
the MediaSDK's ROI configuration.
Signed-off-by: Wenbin Chen
---
libavcodec/qsv_internal.h | 4 ++
libavcodec/qsvenc.c | 86 +++
2 files changed, 90 insertions(+)
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index e2aecdcbd6..8131acdae9 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -51,6 +51,9 @@
#define ASYNC_DEPTH_DEFAULT 4 // internal parallelism
#define QSV_MAX_ENC_PAYLOAD 2 // # of mfxEncodeCtrl payloads supported
+#define QSV_MAX_ENC_EXTPARAM 2
+
+#define QSV_MAX_ROI_NUM 256
#define QSV_MAX_FRAME_EXT_PARAMS 4
@@ -83,6 +86,7 @@ typedef struct QSVFrame {
int num_ext_params;
mfxPayload *payloads[QSV_MAX_ENC_PAYLOAD]; ///< used for enc_ctrl.Payload
+mfxExtBuffer *extparam[QSV_MAX_ENC_EXTPARAM]; ///< used for
enc_ctrl.ExtParam
int queued;
int used;
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 2b3b06767d..0e88a10489 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -1373,15 +1373,29 @@ static void free_encoder_ctrl_payloads(mfxEncodeCtrl*
enc_ctrl)
}
}
+static void free_encoder_ctrl_extparam(mfxEncodeCtrl* enc_ctrl)
+{
+if (enc_ctrl) {
+int i;
+for (i = 0; i < enc_ctrl->NumExtParam && i < QSV_MAX_ENC_EXTPARAM;
i++) {
+if (enc_ctrl->ExtParam[i])
+av_freep(&(enc_ctrl->ExtParam[i]));
+}
+enc_ctrl->NumExtParam = 0;
+}
+}
+
static void clear_unused_frames(QSVEncContext *q)
{
QSVFrame *cur = q->work_frames;
while (cur) {
if (cur->used && !cur->surface.Data.Locked) {
free_encoder_ctrl_payloads(&cur->enc_ctrl);
+free_encoder_ctrl_extparam(&cur->enc_ctrl);
//do not reuse enc_ctrl from previous frame
memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl));
cur->enc_ctrl.Payload = cur->payloads;
+cur->enc_ctrl.ExtParam = cur->extparam;
if (cur->frame->format == AV_PIX_FMT_QSV) {
av_frame_unref(cur->frame);
}
@@ -1419,6 +1433,7 @@ static int get_free_frame(QSVEncContext *q, QSVFrame **f)
return AVERROR(ENOMEM);
}
frame->enc_ctrl.Payload = frame->payloads;
+frame->enc_ctrl.ExtParam = frame->extparam;
*last = frame;
*f = frame;
@@ -1520,6 +1535,68 @@ static void print_interlace_msg(AVCodecContext *avctx,
QSVEncContext *q)
}
}
+static int set_roi_encode_ctrl(AVCodecContext *avctx, const AVFrame *frame,
+ mfxEncodeCtrl *enc_ctrl)
+{
+AVFrameSideData *sd = NULL;
+int mb_size;
+
+if (avctx->codec_id == AV_CODEC_ID_H264) {
+mb_size = 16;
+} else if (avctx->codec_id == AV_CODEC_ID_H265) {
+mb_size = 32;
+} else {
+return 0;
+}
+
+if (frame)
+sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+
+if (sd) {
+mfxExtEncoderROI *enc_roi = NULL;
+AVRegionOfInterest *roi;
+uint32_t roi_size;
+int nb_roi, i;
+
+roi = (AVRegionOfInterest *)sd->data;
+roi_size = roi->self_size;
+if (!roi_size || sd->size % roi_size) {
+av_log(avctx, AV_LOG_ERROR, "Invalid ROI Data.\n");
+return AVERROR(EINVAL);
+}
+nb_roi = sd->size / roi_size;
+if (nb_roi > QSV_MAX_ROI_NUM) {
+av_log(avctx, AV_LOG_WARNING, "More ROIs set than "
+"supported by driver (%d > %d).\n",
+nb_roi, QSV_MAX_ROI_NUM);
+nb_roi = QSV_MAX_ROI_NUM;
+}
+
+enc_roi = av_mallocz(sizeof(*enc_roi));
+if (!enc_roi)
+return AVERROR(ENOMEM);
+enc_roi->Header.BufferId = MFX_EXTBUFF_ENCODER_ROI;
+enc_roi->Header.BufferSz = sizeof(*enc_roi);
+enc_roi->NumROI = nb_roi;
+enc_roi->ROIMode = MFX_ROI_MODE_QP_DELTA;
+for (i = 0; i < nb_roi; i++) {
+roi = (AVRegionOfInterest *)(sd->data + roi_size * i);
+enc_roi->ROI[i].Top= FFALIGN(roi->top, mb_size);
+enc_roi->ROI[i].Bottom = FFALIGN(roi->bottom, mb_size);
+enc_roi->ROI[i].Left = FFALIGN(roi->left, mb_size);
+enc_roi->ROI[i].Right = FFALIGN(roi->right, mb_size);
+enc_roi->ROI[i].DeltaQP =
+roi->qoffset.num * 51 / roi->qoffset.den;
+av_log(avctx, AV_LOG_DEBUG, "ROI: (%d,%d)-(%d,%d) -> %+d.\n",
+ roi->top, roi->left, roi->bottom, roi->right,
+ enc_roi->ROI[i].DeltaQP);
+}
+enc_ctrl->ExtParam[enc_ctrl->NumExtParam] = (mfxExtBuffer *)enc_roi;
+enc_ctrl->NumExtParam++;
+}
+return 0;
+}
+
static int encode_frame(AVCodecContext *avctx, QSVEncContext *q,
Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/vaapi_encode: add support for maxframesize
On Mon, 2022-05-30 at 01:27 +, Xiang, Haihao wrote:
> On Mon, 2022-05-23 at 02:14 +, Wang, Fei W wrote:
> > > -Original Message-
> > > From: ffmpeg-devel On Behalf Of Fei
> > > Wang
> > > Sent: Thursday, May 5, 2022 5:07 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Wang, Fei W ; Linjie Fu
> > > Subject: [FFmpeg-devel] [PATCH v7 1/2] lavc/vaapi_encode: add support for
> > > maxframesize
> > >
> > > From: Linjie Fu
> > >
> > > Add support for max frame size:
> > > - max_frame_size (bytes) to indicate the max allowed size for frame.
> > >
> > > Control each encoded frame size into target limitation size by adjusting
> > > whole
> > > frame's average QP value. The driver will use multi passes to adjust
> > > average
> > > QP
> > > setp by step to achieve the target, and the result may not strictly
> > > guaranteed.
> > > Frame size may exceed target alone with using the maximum average QP
> > > value.
> > > The failure always happens on the intra(especially the first intra frame
> > > of
> > > a new
> > > GOP) frames or set max_frame_size with a very small number.
> > >
> > > example cmdline:
> > > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo \
> > > -v verbose -s:v 352x288 -i ./input.yuv -vf format=nv12,hwupload \
> > > -c:v h264_vaapi -profile:v main -g 30 -rc_mode VBR -b:v 500k \
> > > -bf 3 -max_frame_size 4 -vframes 100 -y ./max_frame_size.h264
> > >
> > > Max frame size was enabled since VA-API version (0, 33, 0), but query is
> > > available
> > > since (1, 5, 0). It will be passed as a parameter in picParam and should
> > > be
> > > set for
> > > each frame.
> > >
> > > Signed-off-by: Linjie Fu
> > > Signed-off-by: Fei Wang
> > > ---
> > > update:
> > > 1. return error when fail to set max frame size.
> > > 2. refine commit and debug message.
> > >
> > > libavcodec/vaapi_encode.c | 74
> > > +++
> > > libavcodec/vaapi_encode.h | 10 +-
> > > 2 files changed, 83 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index
> > > 0e2f5ed447..284ce29888 100644
> > > --- a/libavcodec/vaapi_encode.c
> > > +++ b/libavcodec/vaapi_encode.c
> > > @@ -365,6 +365,17 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
> > > goto fail;
> > > }
> > >
> > > +#if VA_CHECK_VERSION(1, 5, 0)
> > > +if (ctx->max_frame_size) {
> > > +err = vaapi_encode_make_misc_param_buffer(avctx, pic,
> > > + VAEncMiscParameterTypeM
> > > ax
> > > FrameSize,
> > > + &ctx->mfs_params,
> > > + sizeof(ctx-
> > > >mfs_params));
> > > +if (err < 0)
> > > +goto fail;
> > > +}
> > > +#endif
> > > +
> > > if (pic->type == PICTURE_TYPE_IDR) {
> > > if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
> > > ctx->codec->write_sequence_header) { @@ -1869,6 +1880,63 @@
> > > rc_mode_found:
> > > return 0;
> > > }
> > >
> > > +static av_cold int vaapi_encode_init_max_frame_size(AVCodecContext
> > > +*avctx) { #if VA_CHECK_VERSION(1, 5, 0)
> > > +VAAPIEncodeContext *ctx = avctx->priv_data;
> > > +VAConfigAttrib attr = { VAConfigAttribMaxFrameSize };
> > > +VAStatus vas;
> > > +
> > > +if (ctx->va_rc_mode == VA_RC_CQP) {
> > > +ctx->max_frame_size = 0;
> > > +av_log(avctx, AV_LOG_ERROR, "Max frame size is invalid in CQP
> > > rate
> > > "
> > > + "control mode.\n");
> > > +return AVERROR(EINVAL);
> > > +}
> > > +
> > > +vas = vaGetConfigAttributes(ctx->hwctx->display,
> > > +ctx->va_profile,
> > > +ctx->va_entrypoint,
> > > +&attr, 1);
> > > +if (vas != VA_STATUS_SUCCESS) {
> > > +ctx->max_frame_size = 0;
> > > +av_log(avctx, AV_LOG_ERROR, "Failed to query max frame size "
> > > + "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
> > > +return AVERROR_EXTERNAL;
> > > +}
> > > +
> > > +if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
> > > +ctx->max_frame_size = 0;
> > > +av_log(avctx, AV_LOG_ERROR, "Max frame size attribute "
> > > + "is not supported.\n");
> > > +return AVERROR(EINVAL);
> > > +} else {
> > > +VAConfigAttribValMaxFrameSize attr_mfs;
> > > +attr_mfs.value = attr.value;
> > > +// Prefer to use VAEncMiscParameterTypeMaxFrameSize for max frame
> > > size.
> > > +if (!attr_mfs.bits.max_frame_size && attr_mfs.bits.multiple_pass)
> > > {
> > > +ctx->max_frame_size = 0;
> > > +av_log(avctx, AV_LOG_ERROR, "Driver only supports multiple
> > > pass
> > > "
> > > + "max frame size which has not bee
