Added initial version of the transcoding #114
Conversation
t-boiko
force-pushed
the
transcoding_initial
branch
from
d56a0e1 to
cfa8fa8
Compare
t-boiko
force-pushed
the
transcoding_initial
branch
from
cfa8fa8 to
cb4eecf
Compare
| nullptr | ||
| }; | ||
|
|
||
| #if _TRANSCODING |
There was a problem hiding this comment.
If this is the main for transcoding I dont see the point to have #if _TRANSCODING
There was a problem hiding this comment.
The point of this #if _TRANSCODING is a maintenance of the code - with this I will have a clear understanding of which parts of the transcoder's main I should update when the decoder's main is updated
There was a problem hiding this comment.
And a brief explanation: transcoding is an extension of the decoder - it is based on both the decoder and the encoder. Initially, only transcoding from one codec to another is supported (you can specify a target bitrate, a vbv buffer relative size, vbr or cbr as well). But from maintenance and support points of view it would be time consuming to track and update everything from the decoder and the encoder (however the transcoder has it's separate own project folder and Main), thus it is better to put the transcoder specific changes together with the decoder's and the encoder's code. Moreover keeping the transcoding completely separate code would lead to the huge code duplication (even If we are speaking about a separate CMake project - it would duplicate approximately 80-90% of the decoder's project CMake files, moreover it would be required to modify CMake files copied from the decoder and again it would be difficult to maintain the separate CMake project since it will be based on the decoder's CMake project - that's why It would be better to set(_TRANCODING, 1) and pass it using add_subdirectory). My goal was to minimize the code duplication and efforts for the code maintenance and support.
There was a problem hiding this comment.
-> Replacing _TRANCODING by something else like VVS_TRANSCODING
it's OK
-> Other preliminary remark would be to not mix video_decoder and video_transcoder. I would have a separate CMake project for one or the other.
In my opinion, it is better to add small changes (a few lines under the transcoder ifdef) to the decoder CMake file than to copy the same code (3 files with hundreds of lines, that need to be modified and supported) into a new transcoder CMake file and change it there. The transcoder folder consists of CMake file, it is possible to build it using CMake from it's own separate folder (see Build.md).
-> In a next future it would be nice to share common code in a library to avoid the rebuild.
I think it will be difficult to do this as the transcoder will be based on the decoder and encoder folders. It is actually the same code (because of _TRANSCODING passed in CMake, the added code section won't be triggered)
|
Can you please explain briefly in your commit message or the PR what you are trying to achieve here as its not very clear if you want to transcode from one codec to another. (decode->encode) Other preliminary remark would be to not mix video_decoder and video_transcoder. I would have a separate CMake project for one or the other. In a next future it would be nice to share common code in a library to avoid the rebuild. I would replace _TRANCODING by VVS_TRANSCODING or something which identifies that it is a custom project define. |
A brief explanation: transcoding is an extension of the decoder - it is based on both the decoder and the encoder. Initially, only transcoding from one codec to another is supported (you can specify a target bitrate, a vbv buffer relative size, vbr or cbr as well). But from maintenance and support points of view it would be time consuming to track and update everything from the decoder and the encoder (however the transcoder has it's separate own project folder and Main), thus it is better to put the transcoder specific changes together with the decoder's and the encoder's code. Moreover keeping the transcoding completely separate code would lead to the huge code duplication (even If we are speaking about a separate CMake project - it would duplicate approximately 80-90% of the decoder's project CMake files, moreover it would be required to modify CMake files copied from the decoder and again it would be difficult to maintain the separate CMake project since it will be based on the decoder's CMake project - that's why It would be better to set(_TRANCODING, 1) and pass it using add_subdirectory). You can build the transcoder from it's own folder using CMake in the same way as for the decoder or the encoder (see Build.md in the transcoder's folder). My goal was to minimize the code duplication and efforts for the code maintenance and support.