diff mbox

[FFmpeg-devel,1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It does not encode video itself, but rather, this metadata.

Message ID CANcchhfMNqdT=d3scwcubT26ZkozDAi+uVcRuhu3D5RoJjs=ZQ@mail.gmail.com
State New
Headers show

Commit Message

Louis O'Bryan July 12, 2017, 6:31 p.m. UTC
On Wed, Jul 12, 2017 at 9:16 AM, Louis O'Bryan <louiso@google.com> wrote:

> On Wed, Jul 12, 2017 at 12:50 AM, wm4 <nfxjfg@googlemail.com> wrote:
>
>> On Tue, 11 Jul 2017 16:17:33 -0700
>> "Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:
>>
>> > If I need to write a new atom under stsd for my stream in the mov muxer
>> > <https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/movenc.c>
>> > (mov_write_stsd_tag),
>> > is it appropriate to indicate that through the AVStream metadata rather
>> > than the codec_tag?
>>
>> It seemed to have lots of unrelated changes, but maybe I'm missing
>> something. If those codec tag refactors are needed, they should
>> probably be split into a separate patch.
>>
>> But it looks like most of those changes were unintended (Moritz
>> suspected that too). The tag addition itself is probably fine.
>>
>> Also, please don't top post on mailing lists.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> That file had unrelated changes that shouldn't have been there, please
> ignore them.
> Now that there is no codec associated with the stream, there shouldn't be
> a codec tag at all, I would assume. (Another issue I need to deal with is
> that the MOV muxer also doesn't support streams without a codec, but that
> is separate.)
>

My goal is to modify the MOV/MP4 muxer so that I can mux the new stream
with video and audio streams. Part of that is writing a new sample entry
box under the stsd box.
Since I no longer plan to use an encoder for the stream, I was wondering if
the AVStream::metadata
<https://www.ffmpeg.org/doxygen/3.2/structAVStream.html#a50d250a128a3da9ce3d135e84213fb82>
would be an appropriate way to recognize that stream. Other cases in the
mov_write_stsd_tag function use the codec tag.
I have the following sample of that idea here, which allows me to use the
new stream and write the sample entry box:

---
 libavformat/movenc.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

 {
     int64_t pos = avio_tell(pb);
@@ -2077,6 +2094,8 @@ static int mov_write_stsd_tag(AVFormatContext *s,
AVIOContext *pb, MOVMuxContext
         mov_write_rtp_tag(pb, track);
     else if (track->par->codec_tag == MKTAG('t','m','c','d'))
         mov_write_tmcd_tag(pb, track);
+    else if (stream_has_metadata(track->st, "camm"))
+        mov_write_camm_tag(pb);
     return update_size(pb, pos);
 }

@@ -2443,6 +2462,9 @@ static int mov_write_hdlr_tag(AVFormatContext *s,
AVIOContext *pb, MOVTrack *tra
         } else if (track->par->codec_tag == MKTAG('t','m','c','d')) {
             hdlr_type = "tmcd";
             descr = "TimeCodeHandler";
+        } else if (stream_has_metadata(track->st, "camm")) {
+            hdlr_type = "camm";
+            descr = "CameraMetadataMotionHandler";
         } else {
             av_log(s, AV_LOG_WARNING,
                    "Unknown hldr_type for %s, writing dummy values\n",
@@ -5875,7 +5897,7 @@ static int mov_init(AVFormatContext *s)
             track->language = 0;
         track->mode = mov->mode;
         track->tag  = mov_find_codec_tag(s, track);
-        if (!track->tag) {
+        if (!track->tag && !stream_has_metadata(st, "camm")) {
             av_log(s, AV_LOG_ERROR, "Could not find tag for codec %s in
stream #%d, "
                    "codec not currently supported in container\n",
                    avcodec_get_name(st->codecpar->codec_id), i);

Comments

Hendrik Leppkes July 12, 2017, 8:42 p.m. UTC | #1
On Wed, Jul 12, 2017 at 8:31 PM, Louis O'Bryan
<louiso-at-google.com@ffmpeg.org> wrote:
> On Wed, Jul 12, 2017 at 9:16 AM, Louis O'Bryan <louiso@google.com> wrote:
>
>> On Wed, Jul 12, 2017 at 12:50 AM, wm4 <nfxjfg@googlemail.com> wrote:
>>
>>> On Tue, 11 Jul 2017 16:17:33 -0700
>>> "Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:
>>>
>>> > If I need to write a new atom under stsd for my stream in the mov muxer
>>> > <https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/movenc.c>
>>> > (mov_write_stsd_tag),
>>> > is it appropriate to indicate that through the AVStream metadata rather
>>> > than the codec_tag?
>>>
>>> It seemed to have lots of unrelated changes, but maybe I'm missing
>>> something. If those codec tag refactors are needed, they should
>>> probably be split into a separate patch.
>>>
>>> But it looks like most of those changes were unintended (Moritz
>>> suspected that too). The tag addition itself is probably fine.
>>>
>>> Also, please don't top post on mailing lists.
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> That file had unrelated changes that shouldn't have been there, please
>> ignore them.
>> Now that there is no codec associated with the stream, there shouldn't be
>> a codec tag at all, I would assume. (Another issue I need to deal with is
>> that the MOV muxer also doesn't support streams without a codec, but that
>> is separate.)
>>
>
> My goal is to modify the MOV/MP4 muxer so that I can mux the new stream
> with video and audio streams. Part of that is writing a new sample entry
> box under the stsd box.
> Since I no longer plan to use an encoder for the stream, I was wondering if
> the AVStream::metadata
> <https://www.ffmpeg.org/doxygen/3.2/structAVStream.html#a50d250a128a3da9ce3d135e84213fb82>
> would be an appropriate way to recognize that stream. Other cases in the
> mov_write_stsd_tag function use the codec tag.
> I have the following sample of that idea here, which allows me to use the
> new stream and write the sample entry box:
>

You can associate a codec to the stream, put the codec in the data
codec range. You just wouldn't have an encoder for it.
This should probably work without any special hacks, I would guess.

- Hendrik
wm4 July 13, 2017, 8:34 a.m. UTC | #2
On Wed, 12 Jul 2017 22:42:36 +0200
Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Wed, Jul 12, 2017 at 8:31 PM, Louis O'Bryan
> <louiso-at-google.com@ffmpeg.org> wrote:
> > On Wed, Jul 12, 2017 at 9:16 AM, Louis O'Bryan <louiso@google.com> wrote:
> >  
> >> On Wed, Jul 12, 2017 at 12:50 AM, wm4 <nfxjfg@googlemail.com> wrote:
> >>  
> >>> On Tue, 11 Jul 2017 16:17:33 -0700
> >>> "Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:
> >>>  
> >>> > If I need to write a new atom under stsd for my stream in the mov muxer
> >>> > <https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/movenc.c>
> >>> > (mov_write_stsd_tag),
> >>> > is it appropriate to indicate that through the AVStream metadata rather
> >>> > than the codec_tag?  
> >>>
> >>> It seemed to have lots of unrelated changes, but maybe I'm missing
> >>> something. If those codec tag refactors are needed, they should
> >>> probably be split into a separate patch.
> >>>
> >>> But it looks like most of those changes were unintended (Moritz
> >>> suspected that too). The tag addition itself is probably fine.
> >>>
> >>> Also, please don't top post on mailing lists.
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel@ffmpeg.org
> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>  
> >>
> >> That file had unrelated changes that shouldn't have been there, please
> >> ignore them.
> >> Now that there is no codec associated with the stream, there shouldn't be
> >> a codec tag at all, I would assume. (Another issue I need to deal with is
> >> that the MOV muxer also doesn't support streams without a codec, but that
> >> is separate.)
> >>  
> >
> > My goal is to modify the MOV/MP4 muxer so that I can mux the new stream
> > with video and audio streams. Part of that is writing a new sample entry
> > box under the stsd box.
> > Since I no longer plan to use an encoder for the stream, I was wondering if
> > the AVStream::metadata
> > <https://www.ffmpeg.org/doxygen/3.2/structAVStream.html#a50d250a128a3da9ce3d135e84213fb82>
> > would be an appropriate way to recognize that stream. Other cases in the
> > mov_write_stsd_tag function use the codec tag.
> > I have the following sample of that idea here, which allows me to use the
> > new stream and write the sample entry box:
> >  
> 
> You can associate a codec to the stream, put the codec in the data
> codec range. You just wouldn't have an encoder for it.
> This should probably work without any special hacks, I would guess.

If it does, I would assume it uses ffmpeg.c's stream copy path?

I still don't understand where the data comes from. Having a dummy
encoder that consumes dummy AVFrames would also require a dummy
decoder. Unless this is for something outside of ffmpeg.c.
Louis O'Bryan July 13, 2017, 7:05 p.m. UTC | #3
On Thu, Jul 13, 2017 at 1:34 AM, wm4 <nfxjfg@googlemail.com> wrote:

> On Wed, 12 Jul 2017 22:42:36 +0200
> Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> > On Wed, Jul 12, 2017 at 8:31 PM, Louis O'Bryan
> > <louiso-at-google.com@ffmpeg.org> wrote:
> > > On Wed, Jul 12, 2017 at 9:16 AM, Louis O'Bryan <louiso@google.com>
> wrote:
> > >
> > >> On Wed, Jul 12, 2017 at 12:50 AM, wm4 <nfxjfg@googlemail.com> wrote:
> > >>
> > >>> On Tue, 11 Jul 2017 16:17:33 -0700
> > >>> "Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:
> > >>>
> > >>> > If I need to write a new atom under stsd for my stream in the mov
> muxer
> > >>> > <https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/movenc.c
> >
> > >>> > (mov_write_stsd_tag),
> > >>> > is it appropriate to indicate that through the AVStream metadata
> rather
> > >>> > than the codec_tag?
> > >>>
> > >>> It seemed to have lots of unrelated changes, but maybe I'm missing
> > >>> something. If those codec tag refactors are needed, they should
> > >>> probably be split into a separate patch.
> > >>>
> > >>> But it looks like most of those changes were unintended (Moritz
> > >>> suspected that too). The tag addition itself is probably fine.
> > >>>
> > >>> Also, please don't top post on mailing lists.
> > >>> _______________________________________________
> > >>> ffmpeg-devel mailing list
> > >>> ffmpeg-devel@ffmpeg.org
> > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >>>
> > >>
> > >> That file had unrelated changes that shouldn't have been there, please
> > >> ignore them.
> > >> Now that there is no codec associated with the stream, there
> shouldn't be
> > >> a codec tag at all, I would assume. (Another issue I need to deal
> with is
> > >> that the MOV muxer also doesn't support streams without a codec, but
> that
> > >> is separate.)
> > >>
> > >
> > > My goal is to modify the MOV/MP4 muxer so that I can mux the new stream
> > > with video and audio streams. Part of that is writing a new sample
> entry
> > > box under the stsd box.
> > > Since I no longer plan to use an encoder for the stream, I was
> wondering if
> > > the AVStream::metadata
> > > <https://www.ffmpeg.org/doxygen/3.2/structAVStream.html#
> a50d250a128a3da9ce3d135e84213fb82>
> > > would be an appropriate way to recognize that stream. Other cases in
> the
> > > mov_write_stsd_tag function use the codec tag.
> > > I have the following sample of that idea here, which allows me to use
> the
> > > new stream and write the sample entry box:
> > >
> >
> > You can associate a codec to the stream, put the codec in the data
> > codec range. You just wouldn't have an encoder for it.
> > This should probably work without any special hacks, I would guess.
>
> If it does, I would assume it uses ffmpeg.c's stream copy path?
>
> I still don't understand where the data comes from. Having a dummy
> encoder that consumes dummy AVFrames would also require a dummy
> decoder. Unless this is for something outside of ffmpeg.c.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I was using the C API. I am using this code for muxing
<https://github.com/lbobryan/FFmpeg/blob/camm/doc/examples/camm_muxing.c>
and demuxing
<https://github.com/lbobryan/FFmpeg/blob/camm/doc/examples/camm_demuxing.c>.
I have not tried the ffmpeg.c copy path.
For a real use case, the data for the new stream would be coming from a
camera's hardware / sensors as it receives it in real time.
wm4 July 13, 2017, 7:40 p.m. UTC | #4
On Thu, 13 Jul 2017 12:05:15 -0700
"Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:

> On Thu, Jul 13, 2017 at 1:34 AM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> > On Wed, 12 Jul 2017 22:42:36 +0200
> > Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> >  
> > > On Wed, Jul 12, 2017 at 8:31 PM, Louis O'Bryan
> > > <louiso-at-google.com@ffmpeg.org> wrote:  
> > > > On Wed, Jul 12, 2017 at 9:16 AM, Louis O'Bryan <louiso@google.com>  
> > wrote:  
> > > >  
> > > >> On Wed, Jul 12, 2017 at 12:50 AM, wm4 <nfxjfg@googlemail.com> wrote:
> > > >>  
> > > >>> On Tue, 11 Jul 2017 16:17:33 -0700
> > > >>> "Louis O'Bryan" <louiso-at-google.com@ffmpeg.org> wrote:
> > > >>>  
> > > >>> > If I need to write a new atom under stsd for my stream in the mov  
> > muxer  
> > > >>> > <https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/movenc.c  
> > >  
> > > >>> > (mov_write_stsd_tag),
> > > >>> > is it appropriate to indicate that through the AVStream metadata  
> > rather  
> > > >>> > than the codec_tag?  
> > > >>>
> > > >>> It seemed to have lots of unrelated changes, but maybe I'm missing
> > > >>> something. If those codec tag refactors are needed, they should
> > > >>> probably be split into a separate patch.
> > > >>>
> > > >>> But it looks like most of those changes were unintended (Moritz
> > > >>> suspected that too). The tag addition itself is probably fine.
> > > >>>
> > > >>> Also, please don't top post on mailing lists.
> > > >>> _______________________________________________
> > > >>> ffmpeg-devel mailing list
> > > >>> ffmpeg-devel@ffmpeg.org
> > > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >>>  
> > > >>
> > > >> That file had unrelated changes that shouldn't have been there, please
> > > >> ignore them.
> > > >> Now that there is no codec associated with the stream, there  
> > shouldn't be  
> > > >> a codec tag at all, I would assume. (Another issue I need to deal  
> > with is  
> > > >> that the MOV muxer also doesn't support streams without a codec, but  
> > that  
> > > >> is separate.)
> > > >>  
> > > >
> > > > My goal is to modify the MOV/MP4 muxer so that I can mux the new stream
> > > > with video and audio streams. Part of that is writing a new sample  
> > entry  
> > > > box under the stsd box.
> > > > Since I no longer plan to use an encoder for the stream, I was  
> > wondering if  
> > > > the AVStream::metadata
> > > > <https://www.ffmpeg.org/doxygen/3.2/structAVStream.html#  
> > a50d250a128a3da9ce3d135e84213fb82>  
> > > > would be an appropriate way to recognize that stream. Other cases in  
> > the  
> > > > mov_write_stsd_tag function use the codec tag.
> > > > I have the following sample of that idea here, which allows me to use  
> > the  
> > > > new stream and write the sample entry box:
> > > >  
> > >
> > > You can associate a codec to the stream, put the codec in the data
> > > codec range. You just wouldn't have an encoder for it.
> > > This should probably work without any special hacks, I would guess.  
> >
> > If it does, I would assume it uses ffmpeg.c's stream copy path?
> >
> > I still don't understand where the data comes from. Having a dummy
> > encoder that consumes dummy AVFrames would also require a dummy
> > decoder. Unless this is for something outside of ffmpeg.c.
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >  
> 
> I was using the C API. I am using this code for muxing
> <https://github.com/lbobryan/FFmpeg/blob/camm/doc/examples/camm_muxing.c>
> and demuxing
> <https://github.com/lbobryan/FFmpeg/blob/camm/doc/examples/camm_demuxing.c>.
> I have not tried the ffmpeg.c copy path.
> For a real use case, the data for the new stream would be coming from a
> camera's hardware / sensors as it receives it in real time.

Oh I see.

For data verification and dumping, a BSF could be used, btw.
diff mbox

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 88f2f2c819..8d57a18864 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -109,6 +109,13 @@  static const AVClass flavor ## _muxer_class = {\

 static int get_moov_size(AVFormatContext *s);

+static int stream_has_metadata(AVStream *st, const char *metadata_key) {
+    if (!st->metadata) {
+        return 0;
+    }
+    return av_dict_get(st->metadata, metadata_key, NULL, 0) != NULL;
+}
+
 static int utf8len(const uint8_t *b)
 {
     int len = 0;
@@ -2060,6 +2067,16 @@  static int mov_write_tmcd_tag(AVIOContext *pb,
MOVTrack *track)
     return update_size(pb, pos);
 }

+static int mov_write_camm_tag(AVIOContext *pb) {
+    int64_t pos = avio_tell(pb);
+    avio_wb32(pb, 0); /* size */
+    ffio_wfourcc(pb, "camm");
+    avio_wb32(pb, 0); /* Reserved */
+    avio_wb16(pb, 0); /* Reserved */
+    avio_wb16(pb, 1); /* Data-reference index */
+    return update_size(pb, pos);
+}
+
 static int mov_write_stsd_tag(AVFormatContext *s, AVIOContext *pb,
MOVMuxContext *mov, MOVTrack *track)