search

jitsi / libjitsi

Advanced Java media library for secure real-time audio/video communication.
Apache License 2.0
628 stars 281 forks source link

H.263+: Remove non-working setting me_method. #501

Closed traud closed 4 years ago

traud commented 4 years ago

The motion-estimation method (me_method) cannot be set for this encoder, at least not in the default encoder for Debian/Ubuntu. Furthermore, I am not aware of any H.263+ encoder which allows this. FFmpeg ignores unknown options silently. Because FFmpeg 4 moved that option to the private options and removed that public symbol completely, and because that option does not do anything in H.263+, it is simply removed.

jitsi-jenkins commented 4 years ago

Hi, thanks for your contribution! If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

damencho commented 4 years ago

@traud Have you tested h.263, is it working?

traud commented 4 years ago

No, I have not tested the encoder wrapper in LibJitsi for H.263+. I checked the source code of the encoder wrapper in FFmpeg. To double-check, I created a small app in which I tested whether those options are set actually.

damencho commented 4 years ago

I would say, close this PR and create a new one, removing the h263 occurrences. To make things simpler :) the code is there in the history if someone needs it. It is not used let's remove it and not waste time with it. WDYT?

traud commented 4 years ago

Yes, that is fine with me, although many doorbells still offer just H.263+. Anyway, please, do not forget to remove any references in

as well. As an external contributor like me, removing a codec is quite challenging because it is used all over. Consequently, I would prefer a member to do that.

traud commented 4 years ago

Because big changes might never come and this Pull Request does not hurt and this Pull Request block another Pull Request of mine, why not merge this and remove H.263+ later?