[FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)

2022-12-22 Thread Wujian(Chin)


>Wujian(Chin) (12022-12-21):
>> I have modified the issues again. Please review it again. Thank you.
>> 
>> If the protocol address contains the user name and password, the ps 
>> -ef command exposes plaintext. The -mask_url parameter option is added 
>> to replace the protocol address in the command line with the asterisk (*).
>> Because other users can run the ps -ef command to view sensitive 
>> information such as the user name and password in the protocol 
>> address, which is insecure.
>> 
>> Signed-off-by: wujian_nanjing 
>> ---
>>  doc/fftools-common-opts.texi | 11 +++
>>  fftools/cmdutils.c   | 75 
>> ++--
>>  fftools/cmdutils.h   | 25 +++
>>  fftools/ffmpeg.c | 10 +++---
>>  fftools/ffplay.c |  9 --
>>  fftools/ffprobe.c| 10 +++---
>>  6 files changed, 126 insertions(+), 14 deletions(-)

>This new patch still has issues not addressed since the first review.

>Regards,

--
>  Nicolas George

I've modified most of the issues, and I've explained some of the issues that 
don't.
If you don't accept my explanation, do you have any other better suggestions 
and methods?

Thank you.

___
ffmpeg-user mailing list
ffmpeg-u...@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-user

To unsubscribe, visit link above, or email ffmpeg-user-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] libavformat/rtspdec.c: flush pes buffer while rtsp seek

2022-12-22 Thread zhilizhao(赵志立)


> On Dec 22, 2022, at 11:32, tanwei (D)  wrote:
> 
> Fixes ticket #9949.
> 
> 
> Signed-off-by: t00660896 
> 
> ---
> 
> libavformat/mpegts.c| 20 
> 
> libavformat/mpegts.h|  1 +
> 
> libavformat/rtpdec.c|  7 +++
> 
> libavformat/rtpdec.h|  2 ++
> 
> libavformat/rtpdec_mpegts.c | 11 +++
> 
> libavformat/rtspdec.c   | 11 +++
> 
> 6 files changed, 52 insertions(+)
> 
> 

Please check the patch format (a lot of empty lines).

> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> 
> index d97702fcd7..c82971af87 100644
> 
> --- a/libavformat/mpegts.c
> 
> +++ b/libavformat/mpegts.c
> 
> @@ -3419,6 +3419,26 @@ int avpriv_mpegts_parse_packet(MpegTSContext *ts, 
> AVPacket *pkt,
> 
> return len1 - len;
> 
> }
> 
> +void ff_mpegts_seek_flush(MpegTSContext *ts)
> 
> +{
> 
> +int i;
> 
> +/* flush pes buffer */
> 
> +for (i = 0; i < NB_PID_MAX; i++) {
> 
> +if (ts->pids[i]) {
> 
> +if (ts->pids[i]->type == MPEGTS_PES) {
> 
> +PESContext *pes = ts->pids[i]->u.pes_filter.opaque;
> 
> +av_buffer_unref(>buffer);
> 
> +pes->data_index = 0;
> 
> +pes->state = MPEGTS_SKIP; /* skip until pes header */
> 
> +} else if (ts->pids[i]->type == MPEGTS_SECTION) {
> 
> +ts->pids[i]->u.section_filter.last_ver = -1;
> 
> +}
> 
> +ts->pids[i]->last_cc = -1;
> 
> +ts->pids[i]->last_pcr = -1;
> 
> +}
> 
> +}
> 
> +}
> 
> +

Please don’t just duplicate the source code.

> 
> void avpriv_mpegts_parse_close(MpegTSContext *ts)
> 
> {
> 
> mpegts_free(ts);
> 
> diff --git a/libavformat/mpegts.h b/libavformat/mpegts.h
> 
> index a48f14e768..ea6b5106a4 100644
> 
> --- a/libavformat/mpegts.h
> 
> +++ b/libavformat/mpegts.h
> 
> @@ -170,6 +170,7 @@ MpegTSContext *avpriv_mpegts_parse_open(AVFormatContext 
> *s);
> 
> int avpriv_mpegts_parse_packet(MpegTSContext *ts, AVPacket *pkt,
> 
>const uint8_t *buf, int len);
> 
> void avpriv_mpegts_parse_close(MpegTSContext *ts);
> 
> +void ff_mpegts_seek_flush(MpegTSContext *ts);
> 
> typedef struct SLConfigDescr {
> 
> int use_au_start;
> 
> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> 
> index fa7544cc07..d688afd1c1 100644
> 
> --- a/libavformat/rtpdec.c
> 
> +++ b/libavformat/rtpdec.c
> 
> @@ -954,6 +954,13 @@ int ff_rtp_parse_packet(RTPDemuxContext *s, AVPacket 
> *pkt,
> 
> return rv ? rv : has_next_packet(s);
> 
> }
> 
> +void ff_rtp_seek_flush(RTPDemuxContext *s)
> 
> +{
> 
> +ff_rtp_reset_packet_queue(s);
> 
> +if (s->handler && s->handler->seek_flush)
> 
> +s->handler->seek_flush(s->dynamic_protocol_context);
> 
> +}
> 
> +
> 
> void ff_rtp_parse_close(RTPDemuxContext *s)
> 
> {
> 
> ff_rtp_reset_packet_queue(s);
> 
> diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
> 
> index 5a02e72dc2..8d6d857e28 100644
> 
> --- a/libavformat/rtpdec.h
> 
> +++ b/libavformat/rtpdec.h
> 
> @@ -52,6 +52,7 @@ int ff_rtp_parse_packet(RTPDemuxContext *s, AVPacket *pkt,
> 
> void ff_rtp_parse_close(RTPDemuxContext *s);
> 
> int64_t ff_rtp_queued_packet_time(RTPDemuxContext *s);
> 
> void ff_rtp_reset_packet_queue(RTPDemuxContext *s);
> 
> +void ff_rtp_seek_flush(RTPDemuxContext *s);
> 
> /**
> 
>  * Send a dummy packet on both port pairs to set up the connection
> 
> @@ -135,6 +136,7 @@ struct RTPDynamicProtocolHandler {
> 
> /** Parse handler for this dynamic packet */
> 
> DynamicPayloadPacketHandlerProc parse_packet;
> 
> int (*need_keyframe)(PayloadContext *context);
> 
> +void (*seek_flush)(PayloadContext *protocol_data);
> 
> };
> 
> typedef struct RTPPacket {
> 
> diff --git a/libavformat/rtpdec_mpegts.c b/libavformat/rtpdec_mpegts.c
> 
> index 405271f744..46c1d36021 100644
> 
> --- a/libavformat/rtpdec_mpegts.c
> 
> +++ b/libavformat/rtpdec_mpegts.c
> 
> @@ -47,6 +47,16 @@ static av_cold int mpegts_init(AVFormatContext *ctx, int 
> st_index,
> 
> return 0;
> 
> }
> 
> +static void mpegts_seek_flush(PayloadContext *data)
> 
> +{
> 
> +if (!data)
> 
> +return;

Is it possible for data being NULL? It’s better to depends on a clear
lifecycle management rather than NULL pointer check everywhere.

> 
> +memset(data->buf, 0, data->read_buf_size);
> 
> +data->read_buf_size = 0;

What about data->read_buf_index ?

> 
> +if (data->ts)
> 
> +ff_mpegts_seek_flush(data->ts);
> 
> +}
> 
> +
> 
> static int mpegts_handle_packet(AVFormatContext *ctx, PayloadContext *data,
> 
> AVStream *st, AVPacket *pkt, uint32_t 
> *timestamp,
> 
> const uint8_t *buf, int len, uint16_t seq,
> 
> @@ -94,6 +104,7 @@ const RTPDynamicProtocolHandler ff_mpegts_dynamic_handler 
> = {
> 
> .priv_data_size= sizeof(PayloadContext),
> 
> .parse_packet  = mpegts_handle_packet,
> 
> 

Re: [FFmpeg-devel] Would a crypto file be acceptable?

2022-12-22 Thread Mark Gaiser
On Thu, Dec 22, 2022 at 8:15 PM Gregor Riepl  wrote:

> > The result should be no need to provide "crypto://". The ffmpeg file
> format
> > detection should detect that ".crypto" should be handled by the crypto
> > plugin.
>
> Instead of a custom descriptor file format that is only used for this
> particular special case, you could also define a custom URI, such as:
>
>
> encrypted-media://key-format:aes128/key:12345667abcdef/iv:12345678/uri:file%3A%2F%2F%2Ftmp%2Ffile.mp4
>
>
I can't find a single thing about this in the ffmpeg documentation.
How is this called, where can I read more about it and - most importantly -
does it work out of the box?

An option that I'm looking for should work without providing custom
arguments in ffmpeg!

Regardless, this would be a rather big security hole as a potential key
would be plainly visible in url logging.
Therefore "hiding" it in a file is probably a better and more secure
approach.


> ___
> 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 2/3] avcodec/mjpegbdec: Check for AVDISCARD_ALL

2022-12-22 Thread Michael Niedermayer
On Sun, Nov 13, 2022 at 12:44:00AM +0100, Michael Niedermayer wrote:
> Fixes: Assertion failure
> Fixes: 
> 51825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MJPEGB_fuzzer-6393802688692224
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/mjpegbdec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
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/3] avformat/lafdec: Check for EOF in header reading

2022-12-22 Thread Michael Niedermayer
On Sun, Nov 13, 2022 at 12:43:59AM +0100, Michael Niedermayer wrote:
> Fixes: OOM testcase
> Fixes: 
> 51527/clusterfuzz-testcase-minimized-ffmpeg_dem_LAF_fuzzer-5453663505612800
> 
> OOM can still happen after this as an arbitrary sized block is allocated and 
> read
> this would require a redesign or some limit on the sample rate.
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/lafdec.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle


signature.asc
Description: PGP signature
___
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 3/3] avformat/mxfdec: Reduce overflows in essence_length computation

2022-12-22 Thread Michael Niedermayer
On Wed, Nov 16, 2022 at 12:49:10PM +0100, Tomas Härdin wrote:
> sön 2022-11-13 klockan 00:44 +0100 skrev Michael Niedermayer:
> > Fixes: signed integer overflow: -3741319169 - 9223372036823449370
> > cannot be represented in type 'long'
> > Fixes: 51896/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> > 513039428681728
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/mxfdec.c | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index e6118e141d..42109cb43a 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -100,7 +100,7 @@ typedef struct MXFPartition {
> >  uint64_t previous_partition;
> >  int index_sid;
> >  int body_sid;
> > -    int64_t this_partition;
> > +    uint64_t this_partition;
> >  int64_t essence_offset; ///< absolute offset of essence
> >  int64_t essence_length;
> >  int32_t kag_size;
> > @@ -3519,8 +3519,12 @@ static void
> > mxf_compute_essence_containers(AVFormatContext *s)
> >  p->essence_offset = p->first_essence_klv.offset;
> >  
> >  /* essence container spans to the next partition */
> > -    if (x < mxf->partitions_count - 1)
> > -    p->essence_length = mxf-
> > >partitions[x+1].this_partition - p->essence_offset;
> > +    if (x < mxf->partitions_count - 1) {
> > +    if (mxf->partitions[x+1].this_partition < p-
> > >essence_offset) {
> > +    p->essence_length = -1;
> > +    } else
> > +    p->essence_length = mxf-
> > >partitions[x+1].this_partition - p->essence_offset;
> > +    }
> 
> A better solution might be to record the actual offset of the
> partitions rather than relying on ThisPartition. Then we can guarantee
> that they are strictly increasing.

do you mean that pack_ofs could be used here ?
(this is within 8 byte of this_partition) for the files i checked
or something else ?

but as pack_ofs is signed that either means dealing with potentially negative
or changing it to uint64_t or adding a new field and avio_tell() somewhere?

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is a danger to trust the dream we wish for rather than
the science we have, -- Dr. Kenneth Brown


signature.asc
Description: PGP signature
___
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] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)

2022-12-22 Thread Marton Balint



On Mon, 19 Dec 2022, "zhilizhao(赵志立)" wrote:





On Dec 19, 2022, at 21:40, Marvin Scholz  wrote:


On 19 Dec 2022, at 14:37, Nicolas George wrote:


Marvin Scholz (12022-12-19):

IIUC this means the `-mask_url` option has to be the first option passed,
which seems a bit of an unfortunate requirement and is not documented at
all, as far as I can see. So at least this should be clearly documented
to prevent users being confused why the get an unrecognised option error
when they do not pass it as the first option.


Indeed. And I see no reason to have this option processed specially like
that; it requires at least an explanation.


I am a bit confused how this helps for the issue it tries to solve, as
for some amount of time, until this is done, it would expose the full
plaintext URL still, no?


This is unavoidable. Still, having sensitive information visible for a
fraction of a second is better than having sensitive information visible
for the length of a playback or transcoding process.


I agree, but then the docs should probably mention that to not give a false
sense of absolute security here. And maybe note that it might
be a better option to pass the password via stdin or hide the process
from other users to completely avoid leaking the password.


We have options like ‘-password', ‘-key’, ‘-cryptokey' and so on. I prefer 
hide the entire argument lists if we accept this solution. I don’t know about

system administration, hidepid looks like a neat solution.
https://linux-audit.com/linux-system-hardening-adding-hidepid-to-proc/


I am not a fan of this masking, because the false sense of security, docs 
or not. Does wget or curl mask its command line?


But I agree, if such "feature" is added, it should remove the whole 
command line. And the docs should point to real solutions, like hidepid.


Regards,
Marton
___
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] avformat/rtpproto: add support for RTP/UDP socket reuse

2022-12-22 Thread Nicolas George
Camille Oudot (12022-12-22):
> This patch introduces a "reuse" option over the RTP protocol. It simply
> passes the value to the underlying UDP protocol's "reuse" option.
> 
> Some RTP peers expect multiple streams to come from the same IP/port, e.g.
> when RTP BUNDLE is involved (different streams sent from/to the same
> srcIP/srcPort/dspIp/dspPort tuple), or when rtcp-mux is involved (RTP and
> RTCP packets are muxed together).

It is not your fault, but the name of the option is misleading: it does
not reuse any sockets, it allows to reuse an *address* and maps to the
SO_REUSEADDR socket option. The misleading part is already in udp.c, but
I think we should avoid letting it proliferate.

Also, instead of passing this particular option, I think it would be
better to make the UDP context a child context of the RTP context, so
that options are applied recursively automatically.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)

2022-12-22 Thread Nicolas George
Wujian(Chin) (12022-12-21):
> I have modified the issues again. Please review it again. Thank you.
> 
> If the protocol address contains the user name and password, the ps -ef
> command exposes plaintext. The -mask_url parameter option is added to
> replace the protocol address in the command line with the asterisk (*).
> Because other users can run the ps -ef command to view sensitive
> information such as the user name and password in the protocol address,
> which is insecure.
> 
> Signed-off-by: wujian_nanjing 
> ---
>  doc/fftools-common-opts.texi | 11 +++
>  fftools/cmdutils.c   | 75 
> ++--
>  fftools/cmdutils.h   | 25 +++
>  fftools/ffmpeg.c | 10 +++---
>  fftools/ffplay.c |  9 --
>  fftools/ffprobe.c| 10 +++---
>  6 files changed, 126 insertions(+), 14 deletions(-)

This new patch still has issues not addressed since the first review.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)

2022-12-22 Thread Nicolas George
Wujian(Chin) (12022-12-20):
>   I think that it's more concise to use code this way.

Concision is not the goal here, maintainability is. Please do not use
gotos.

>  I think that it would be better to replace the entire url, so that the code 
> implementation is simple.

Then replace the whole command line, it is even simpler. Also, this way
you miss credentials passed through options.

> >> +argv2 = av_mallocz(argc * sizeof(char *));
> 
> >sizeof(*argv2)

Youhoud?

>  This option needs to replace the URL. It is more appropriate to judge
>  mask_url and copy argv in this place. Otherwise, do you have any
>  other suggestions?

Use the normal options parsing system.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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] Would a crypto file be acceptable?

2022-12-22 Thread Gregor Riepl

The result should be no need to provide "crypto://". The ffmpeg file format
detection should detect that ".crypto" should be handled by the crypto
plugin.


Instead of a custom descriptor file format that is only used for this 
particular special case, you could also define a custom URI, such as:


encrypted-media://key-format:aes128/key:12345667abcdef/iv:12345678/uri:file%3A%2F%2F%2Ftmp%2Ffile.mp4

___
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] Would a crypto file be acceptable?

2022-12-22 Thread Hendrik Leppkes
On Thu, Dec 22, 2022 at 4:53 PM Mark Gaiser  wrote:
>
> On Thu, Dec 22, 2022 at 11:40 AM Nicolas George  wrote:
>
> > Mark Gaiser (12022-12-21):
> > > While this works just fine, it's limited in use because the cryptography
> > > details have to be passed on the command line. Applications that might
> > well
> > > support much of ffmpeg functionality can't easily hook into the crypto
> > > functionality. Take KODI for example, it allows playback of many of the
> > > formats ffmpeg supports but anything with crypto just isn't possible. In
> > > fact, anything that requires custom command line arguments isn't
> > possible.
> > > [2]
> > >
> > > My idea is to make a new file format that would be implemented and
> > specced
> > > within [1]. My proposed format would be:
> > >
> > > ---
> > > CRYPTO-VERSION:1
> > > CRYPTO-KEY:URI:.
> > > CRYPTO-IV:URI:.
> > > encrypted_file
> > > ---
> >
> > The concat demuxer can already contain options, and despite is name it
> > can be used with a single file.
> >
>
> Could you elaborate on how to use that:?
>
> The end result needs to be:
> ffplay 
>
> that needs to translate to:
> ffplay crypto://encrypted_file -decryption_key $AES_KEY -decryption_iv
> $AES_IV
>
> I briefly looked at the concat demuxer but couldn't see how to get this
> desired result.
> If you know how, please let me know!
>

Create a file like this:
https://pastebin.com/hFSeXsZt

The first line is so ffmpeg can probe the format, then just have a
"file" line followed by any number of "option" lines.

Note that options are only allowed if safe-mode is disengaged, so the
app in question would have to pass "-safe 0" to the execution in some
manner. But this is something you would have to ask the app to do, as
this protection exists for a reason.

- Hendrik
___
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] Would a crypto file be acceptable?

2022-12-22 Thread Mark Gaiser
On Thu, Dec 22, 2022 at 11:40 AM Nicolas George  wrote:

> Mark Gaiser (12022-12-21):
> > While this works just fine, it's limited in use because the cryptography
> > details have to be passed on the command line. Applications that might
> well
> > support much of ffmpeg functionality can't easily hook into the crypto
> > functionality. Take KODI for example, it allows playback of many of the
> > formats ffmpeg supports but anything with crypto just isn't possible. In
> > fact, anything that requires custom command line arguments isn't
> possible.
> > [2]
> >
> > My idea is to make a new file format that would be implemented and
> specced
> > within [1]. My proposed format would be:
> >
> > ---
> > CRYPTO-VERSION:1
> > CRYPTO-KEY:URI:.
> > CRYPTO-IV:URI:.
> > encrypted_file
> > ---
>
> The concat demuxer can already contain options, and despite is name it
> can be used with a single file.
>

Could you elaborate on how to use that:?

The end result needs to be:
ffplay 

that needs to translate to:
ffplay crypto://encrypted_file -decryption_key $AES_KEY -decryption_iv
$AES_IV

I briefly looked at the concat demuxer but couldn't see how to get this
desired result.
If you know how, please let me know!



>
> Regards,
>
> --
>   Nicolas George
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-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] avformat/rtpproto: add support for RTP/UDP socket reuse

2022-12-22 Thread Camille Oudot
Re-submitting because google webmail did not set the correct mime type,
sorry for the noise.

-- 
Camille
From 998e1d3d79b416422e2b1d4f9a5ffb92062db256 Mon Sep 17 00:00:00 2001
From: Camille Oudot 
Date: Fri, 16 Dec 2022 15:30:02 +0100
Subject: [PATCH] avformat/rtpproto: add support for RTP/UDP socket reuse

This patch introduces a "reuse" option over the RTP protocol. It simply
passes the value to the underlying UDP protocol's "reuse" option.

Some RTP peers expect multiple streams to come from the same IP/port, e.g.
when RTP BUNDLE is involved (different streams sent from/to the same
srcIP/srcPort/dspIp/dspPort tuple), or when rtcp-mux is involved (RTP and
RTCP packets are muxed together).

This patch allows ffmpeg to bundle RTP streams and mux RTP/RTCP together by
setting the "reuse" option, and fiddling with the "localaddr", "localport",
"localrtcpport" and "rtcpport" options.

Signed-off-by: Camille Oudot 
---
 Changelog  |  1 +
 libavformat/rtpproto.c | 11 +++
 2 files changed, 12 insertions(+)

diff --git a/Changelog b/Changelog
index f3a6abb9cd..39b68f1702 100644
--- a/Changelog
+++ b/Changelog
@@ -28,6 +28,7 @@ version :
 - showcwt multimedia filter
 - corr video filter
 - adrc audio filter
+- Add RTP protocol "reuse" option to allow UDP socket reuse
 
 
 version 5.1:
diff --git a/libavformat/rtpproto.c b/libavformat/rtpproto.c
index b970901d01..e135907127 100644
--- a/libavformat/rtpproto.c
+++ b/libavformat/rtpproto.c
@@ -55,6 +55,7 @@ typedef struct RTPContext {
 int buffer_size;
 int rtcp_port, local_rtpport, local_rtcpport;
 int connect;
+int reuse_socket;
 int pkt_size;
 int dscp;
 char *sources;
@@ -74,6 +75,7 @@ static const AVOption options[] = {
 { "local_rtpport",  "Local rtp port",   OFFSET(local_rtpport),   AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "local_rtcpport", "Local rtcp port",  OFFSET(local_rtcpport),  AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "connect","Connect socket",   OFFSET(connect), AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
+{ "reuse",  "Explicitly allow reusing UDP sockets", OFFSET(reuse_socket),AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
 { "write_to_source","Send packets to the source address of the latest received packet", OFFSET(write_to_source), AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
 { "pkt_size",   "Maximum packet size",  OFFSET(pkt_size),AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "dscp",   "DSCP class",   OFFSET(dscp),AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
@@ -191,6 +193,8 @@ static void build_udp_url(RTPContext *s,
 url_add_option(buf, buf_size, "pkt_size=%d", s->pkt_size);
 if (s->connect)
 url_add_option(buf, buf_size, "connect=1");
+if (s->reuse_socket)
+url_add_option(buf, buf_size, "reuse=1");
 if (s->dscp >= 0)
 url_add_option(buf, buf_size, "dscp=%d", s->dscp);
 url_add_option(buf, buf_size, "fifo_size=0");
@@ -266,6 +270,13 @@ static int rtp_open(URLContext *h, const char *uri, int flags)
 if (av_find_info_tag(buf, sizeof(buf), "connect", p)) {
 s->connect = strtol(buf, NULL, 10);
 }
+if (av_find_info_tag(buf, sizeof(buf), "reuse", p)) {
+char *endptr = NULL;
+s->reuse_socket = strtol(buf, , 10);
+/* assume if no digits were found it is a request to enable it */
+if (buf == endptr)
+s->reuse_socket = 1;
+}
 if (av_find_info_tag(buf, sizeof(buf), "write_to_source", p)) {
 s->write_to_source = strtol(buf, NULL, 10);
 }
-- 
2.30.2

___
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] avformat/rtpproto: add support for RTP/UDP socket reuse

2022-12-22 Thread Camille Oudot

From 998e1d3d79b416422e2b1d4f9a5ffb92062db256 Mon Sep 17 00:00:00 2001
From: Camille Oudot 
Date: Fri, 16 Dec 2022 15:30:02 +0100
Subject: [PATCH] avformat/rtpproto: add support for RTP/UDP socket reuse

This patch introduces a "reuse" option over the RTP protocol. It simply
passes the value to the underlying UDP protocol's "reuse" option.

Some RTP peers expect multiple streams to come from the same IP/port, e.g.
when RTP BUNDLE is involved (different streams sent from/to the same
srcIP/srcPort/dspIp/dspPort tuple), or when rtcp-mux is involved (RTP and
RTCP packets are muxed together).

This patch allows ffmpeg to bundle RTP streams and mux RTP/RTCP together by
setting the "reuse" option, and fiddling with the "localaddr", "localport",
"localrtcpport" and "rtcpport" options.

Signed-off-by: Camille Oudot 
---
 Changelog  |  1 +
 libavformat/rtpproto.c | 11 +++
 2 files changed, 12 insertions(+)

diff --git a/Changelog b/Changelog
index f3a6abb9cd..39b68f1702 100644
--- a/Changelog
+++ b/Changelog
@@ -28,6 +28,7 @@ version :
 - showcwt multimedia filter
 - corr video filter
 - adrc audio filter
+- Add RTP protocol "reuse" option to allow UDP socket reuse
 
 
 version 5.1:
diff --git a/libavformat/rtpproto.c b/libavformat/rtpproto.c
index b970901d01..e135907127 100644
--- a/libavformat/rtpproto.c
+++ b/libavformat/rtpproto.c
@@ -55,6 +55,7 @@ typedef struct RTPContext {
 int buffer_size;
 int rtcp_port, local_rtpport, local_rtcpport;
 int connect;
+int reuse_socket;
 int pkt_size;
 int dscp;
 char *sources;
@@ -74,6 +75,7 @@ static const AVOption options[] = {
 { "local_rtpport",  "Local rtp port",   OFFSET(local_rtpport),   AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "local_rtcpport", "Local rtcp port",  OFFSET(local_rtcpport),  AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "connect","Connect socket",   OFFSET(connect), AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
+{ "reuse",  "Explicitly allow reusing UDP sockets", OFFSET(reuse_socket),AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
 { "write_to_source","Send packets to the source address of the latest received packet", OFFSET(write_to_source), AV_OPT_TYPE_BOOL,   { .i64 =  0 }, 0, 1,   .flags = D|E },
 { "pkt_size",   "Maximum packet size",  OFFSET(pkt_size),AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
 { "dscp",   "DSCP class",   OFFSET(dscp),AV_OPT_TYPE_INT,{ .i64 = -1 },-1, INT_MAX, .flags = D|E },
@@ -191,6 +193,8 @@ static void build_udp_url(RTPContext *s,
 url_add_option(buf, buf_size, "pkt_size=%d", s->pkt_size);
 if (s->connect)
 url_add_option(buf, buf_size, "connect=1");
+if (s->reuse_socket)
+url_add_option(buf, buf_size, "reuse=1");
 if (s->dscp >= 0)
 url_add_option(buf, buf_size, "dscp=%d", s->dscp);
 url_add_option(buf, buf_size, "fifo_size=0");
@@ -266,6 +270,13 @@ static int rtp_open(URLContext *h, const char *uri, int flags)
 if (av_find_info_tag(buf, sizeof(buf), "connect", p)) {
 s->connect = strtol(buf, NULL, 10);
 }
+if (av_find_info_tag(buf, sizeof(buf), "reuse", p)) {
+char *endptr = NULL;
+s->reuse_socket = strtol(buf, , 10);
+/* assume if no digits were found it is a request to enable it */
+if (buf == endptr)
+s->reuse_socket = 1;
+}
 if (av_find_info_tag(buf, sizeof(buf), "write_to_source", p)) {
 s->write_to_source = strtol(buf, NULL, 10);
 }
-- 
2.30.2

___
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] avformat/mov: fix buffering issue for special HTTP(s) mp4.

2022-12-22 Thread Zhao Zhili
On Tue, 2022-12-20 at 09:20 +, Chen, Jinkai wrote:
> Problem:
> Using ffplay play on these sources:
> https://ali-sprite-video.yyouwang.com/video/works/202211/1667997073624_73.mp4
> https://images.voghion.com/productImages/04_01_C_30011_2020220106GiuseppeFanara0012.mp4
> 
> Solution:
> Add a private option, it will use separated IO context(HTTP
> connection)
> for each AVStream. Preventing from reading audio and video in
> long distance(offset), which cause seeking(HTTP request) frequently.
> 
> Storing the user options when open input,
> and make sure that can be passed to demuxer context.

I'm afraid such workaround is difficult to maintain for long term. You
can achieve the same result (with a little overhead) with multiple
AVFormatContexts, which is still abuse of network transport. The only
elegant way I can get to handle such broken files is download to local
storage and remux.

> 
> Signed-off-by: Gamhoi Chan  chenjin...@agora.io>>
> ---
> libavformat/avformat.c |  1 +
> libavformat/demux.c|  5 -
> libavformat/internal.h |  5 +
> libavformat/isom.h |  1 +
> libavformat/mov.c  | 14 +-
> 5 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.c b/libavformat/avformat.c
> index 19c7219471..4453727f34 100644
> --- a/libavformat/avformat.c
> +++ b/libavformat/avformat.c
> @@ -129,6 +129,7 @@ void avformat_free_context(AVFormatContext *s)
> av_freep(>chapters);
> av_dict_free(>metadata);
> av_dict_free(>id3v2_meta);
> +av_dict_free(>options);
> av_packet_free(>pkt);
> av_packet_free(>parse_pkt);
> av_freep(>streams);
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 2dfd82a63c..2377bfdab0 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -237,8 +237,11 @@ int avformat_open_input(AVFormatContext **ps,
> const char *filename,
> if (fmt)
> s->iformat = fmt;
> 
> -if (options)
> +if (options) {
> av_dict_copy(, *options, 0);
> +si->options = NULL;
> +av_dict_copy(>options, *options, 0);
> +}
> 
> if (s->pb) // must be before any goto fail
> s->flags |= AVFMT_FLAG_CUSTOM_IO;
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index ce837fefc7..7caae8b93e 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -186,6 +186,11 @@ typedef struct FFFormatContext {
>  * Contexts and child contexts do not contain a metadata option
>  */
> int metafree;
> +
> +/**
> + * options from avformat_open_input
> + */
> +AVDictionary *options;
> } FFFormatContext;
> 
> static av_always_inline FFFormatContext
> *ffformatcontext(AVFormatContext *s)
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 64fb7065d5..dad049a2df 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -326,6 +326,7 @@ typedef struct MOVContext {
> int64_t extent_offset;
> } *avif_info;
> int avif_info_size;
> +int use_stream_pb;
> } MOVContext;
> 
> int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 29bd3103e3..0d5818b327 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4519,6 +4519,18 @@ static int mov_read_trak(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>st->index, dref->path, dref->dir, dref->filename,
>dref->volume, dref->nlvl_from, dref->nlvl_to);
> }
> +} else if (c->use_stream_pb) {
> +FFFormatContext *const si = ffformatcontext(c->fc);
> +AVDictionary *opts = NULL;
> +av_dict_copy(, si->options, 0);
> +ret = c->fc->io_open(c->fc, >pb, c->fc->url,
> AVIO_FLAG_READ, );
> +av_dict_free();
> +if (ret < 0) {
> +av_log(c->fc, AV_LOG_ERROR,
> +"use_stream_pb, stream %d, error opening url %s.\n",
> +st->index, c->fc->url);
> +return ret;
> +}
> } else {
> sc->pb = c->fc->pb;
> sc->pb_is_copied = 1;
> @@ -9119,7 +9131,7 @@ static const AVOption mov_options[] = {
> { "enable_drefs", "Enable external track support.",
> OFFSET(enable_drefs), AV_OPT_TYPE_BOOL,
> {.i64 = 0}, 0, 1, FLAGS },
> { "max_stts_delta", "treat offsets above this value as invalid",
> OFFSET(max_stts_delta), AV_OPT_TYPE_INT, {.i64 = UINT_MAX-48000*10 },
> 0, UINT_MAX, .flags = AV_OPT_FLAG_DECODING_PARAM },
> -
> +{ "use_stream_pb", "Each steam has its own AVIOContext.",
> OFFSET(use_stream_pb), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS },
> { NULL },
> };
> 
> --
> ___
> 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

Re: [FFmpeg-devel] Would a crypto file be acceptable?

2022-12-22 Thread Nicolas George
Mark Gaiser (12022-12-21):
> While this works just fine, it's limited in use because the cryptography
> details have to be passed on the command line. Applications that might well
> support much of ffmpeg functionality can't easily hook into the crypto
> functionality. Take KODI for example, it allows playback of many of the
> formats ffmpeg supports but anything with crypto just isn't possible. In
> fact, anything that requires custom command line arguments isn't possible.
> [2]
> 
> My idea is to make a new file format that would be implemented and specced
> within [1]. My proposed format would be:
> 
> ---
> CRYPTO-VERSION:1
> CRYPTO-KEY:URI:.
> CRYPTO-IV:URI:.
> encrypted_file
> ---

The concat demuxer can already contain options, and despite is name it
can be used with a single file.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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".