Skip to content

Commit

Permalink
FormatContext: add flag parameter to the seek function
Browse files Browse the repository at this point in the history
  • Loading branch information
Clement Champetier committed Apr 22, 2015
1 parent 32d99fd commit 1443a1a
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/AvTranscoder/file/FormatContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ AVStream& FormatContext::addAVStream( const AVCodec& avCodec )
return *stream;
}

void FormatContext::seek( uint64_t position )
void FormatContext::seek( uint64_t position, const int flag )
{
if( (int)getStartTime() != AV_NOPTS_VALUE )
position += getStartTime();

if( av_seek_frame( _avFormatContext, -1, position, AVSEEK_FLAG_BACKWARD ) < 0 )
if( av_seek_frame( _avFormatContext, -1, position, flag ) < 0 )
{
LOG_ERROR( "Error when seek at " << position << " (in AV_TIME_BASE units) in file" )
}
Expand Down
7 changes: 4 additions & 3 deletions src/AvTranscoder/file/FormatContext.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ class AvExport FormatContext
AVStream& addAVStream( const AVCodec& avCodec );

/**
* @brief Seek at a specific position (in AV_TIME_BASE units)
* @brief Seek at a specific position
* @param position: can be in AV_TIME_BASE units, in frames... depending on the flag value
* @param flag: seeking mode (AVSEEK_FLAG_xxx)
* @note before seek, add offset of start time
* @note after seek, clear buffering of streams
*/
void seek( uint64_t position );
void seek( uint64_t position, const int flag );

size_t getNbStreams() const { return _avFormatContext->nb_streams; }
/// Get duration of the program, in seconds
Expand Down
4 changes: 2 additions & 2 deletions src/AvTranscoder/file/InputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ bool InputFile::readNextPacket( CodedData& data, const size_t streamIndex )
void InputFile::seekAtFrame( const size_t frame )
{
uint64_t position = frame / getFps() * AV_TIME_BASE;
_formatContext.seek( position );
_formatContext.seek( position, AVSEEK_FLAG_BACKWARD );
}

void InputFile::seekAtTime( const double time )
{
uint64_t position = time * AV_TIME_BASE;
_formatContext.seek( position );
_formatContext.seek( position, AVSEEK_FLAG_BACKWARD );
}

void InputFile::activateStream( const size_t streamIndex, bool activate )
Expand Down
4 changes: 2 additions & 2 deletions src/AvTranscoder/mediaProperty/FileProperties.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void FileProperties::extractStreamProperties( IProgress& progress, const EAnalys

// if the analysis level wiil decode some streams parts, seek at the beginning
if( level > eAnalyseLevelHeader )
const_cast<FormatContext*>( _formatContext )->seek( 0 );
const_cast<FormatContext*>( _formatContext )->seek( 0, AVSEEK_FLAG_BACKWARD );

for( size_t streamIndex = 0; streamIndex < _formatContext->getNbStreams(); ++streamIndex )
{
Expand Down Expand Up @@ -88,7 +88,7 @@ void FileProperties::extractStreamProperties( IProgress& progress, const EAnalys

// if the analysis level has decoded some streams parts, return at the beginning
if( level > eAnalyseLevelHeader )
const_cast<FormatContext*>( _formatContext )->seek( 0 );
const_cast<FormatContext*>( _formatContext )->seek( 0, AVSEEK_FLAG_BACKWARD );
}

std::string FileProperties::getFilename() const
Expand Down

8 comments on commit 1443a1a

@MarcAntoine-Arnaud
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add an enum to do that ?
It's very FFMpeg dependent code ....

@cchampet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FormatContext is ffmpeg dependant, so I don't think it's a problem (for now, with only ffmpeg/libav used by avtranscoder).
But we paid attention of the InputFile class, and so it doesn't manipulate ffmpeg seek flags: it only uses seekAtFrame and seekAtTime.
What do you think of this solution?

@MarcAntoine-Arnaud
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it better to have this method:

void seek( const uint64_t position, const ESeekFlag flag );

@cchampet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where should we define this function?
Libav/ffmpeg have defined seek flags, and we use them. I don't want to re-make some work already done... And this is not incompatible with a generic API which can fit with several decoding libraries. A new enum like ESeekFlag will not enable us to skip a call somewhere to av_seek_frame with the correct ffmpeg flag in FormatContext class.
Don't you think?

@MarcAntoine-Arnaud
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have add a method who pass an simple int ...
void FormatContext::seek( uint64_t position, const int flag )
I think it's better to pass an enum here ...

@cchampet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we are talking about different things.
You want to replace FormatContext::seek by

void FormatContext::seek( uint64_t position, const ESeekFlag flag )
{
        swtich(flag)
        {
                case BACKWARD:
                {
                        av_seek_frame( _avFormatContext, -1, position, AVSEEK_FLAG_BACKWARD )
                        break;
                }
                case ...
                ............
        }
}

?

@MarcAntoine-Arnaud
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes something like that.

void FormatContext::seek( uint64_t position, const ESeekFlag flag )
{
        int ffmpegSeekFlag = AVSEEK_FLAG_BACKWARD;
        switch(flag)
        {
                case BACKWARD:
                {
                        ffmpegSeekFlag = AVSEEK_FLAG_BACKWARD;
                        break;
                }
                case ...
                ............
        }
        av_seek_frame( _avFormatContext, -1, position, ffmpegSeekFlag )
}

@valnoel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nowadays, InputFile and FormatContext classes depend directly on FFmpeg/LibAv. Maybe the first step would be to create an virtual base class (like IInputFile), which would be FFmpeg independent !
This class could allow to define a new way (methods) to seek into the input files, with the specific flags, etc...
#184

Please sign in to comment.