Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

video encoders should never be closed whilst encoding is in progress #865

Closed
totaam opened this issue May 21, 2015 · 3 comments
Closed

video encoders should never be closed whilst encoding is in progress #865

totaam opened this issue May 21, 2015 · 3 comments

Comments

@totaam
Copy link
Collaborator

totaam commented May 21, 2015

Issue migrated from trac ticket # 865

component: server | priority: critical | resolution: fixed

2015-05-21 04:22:20: antoine created the issue


Found in [/attachment/ticket/863/xpra863encoding(1).txt]:

Traceback (most recent call last):
1245	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 958, in delayed_region_soft_timeout
1246	    self.do_send_delayed()
1247	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1048, in do_send_delayed
1248	    self.send_delayed_regions(*delayed)
1249	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1063, in send_delayed_regions
1250	    self.do_send_delayed_regions(damage_time, window, regions, coding, options)
1251	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_video_source.py", line 552, in do_send_delayed_regions
1252	    WindowSource.do_send_delayed_regions(self, damage_time, window, regions, coding, options)
1253	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1098, in do_send_delayed_regions
1254	    send_full_window_update()
1255	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1077, in send_full_window_update
1256	    self.process_damage_region(damage_time, window, 0, 0, ww, wh, actual_encoding, options)
1257	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_video_source.py", line 632, in process_damage_region
1258	    WindowSource.process_damage_region(self, damage_time, window, x, y, w-dw, h-dh, coding, options)
1259	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1199, in process_damage_region
1260	    self.make_data_packet_cb(*item)
1261	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1261, in make_data_packet_cb
1262	    packet = self.make_data_packet(damage_time, process_damage_time, wid, image, coding, sequence, options)
1263	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_source.py", line 1601, in make_data_packet
1264	    ret = encoder(coding, image, options)
1265	  File "/usr/lib64/python2.7/site-packages/xpra/server/window_video_source.py", line 1267, in video_encode
1266	    ret = self._video_encoder.compress_image(csc_image, quality, speed, options)
1267	  File "xpra/codecs/enc_x264/encoder.pyx", line 549, in xpra.codecs.enc_x264.encoder.Encoder.compress_image (xpra/codecs/enc_x264/encoder.c:6237)
1268	AttributeError: 'int' object has no attribute 'append'

The error shown is because of another minor bug fixes in r9460. (backport unimportant)

The real issue is that this bug can only trigger if we finish compressing the pixture with x264 after the encoder context has been freed, which is illegal.

The encoder context can be freed:

  • when garbage collected (should not happen)
  • explicitly when we call encoder.clean() which is called from:
  • WindowVideoSource.video_encoder_clean via the encode thread, so this is safe
  • WindowVideoSource.do_video_encoder_clean:
  • check_pipeline and setup_pipeline which run in the encode thread

Looks fine, but the stacktrace does not lie!

@totaam
Copy link
Collaborator Author

totaam commented May 21, 2015

2015-05-21 04:39:24: antoine changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented May 21, 2015

2015-05-21 04:39:24: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented May 21, 2015

2015-05-21 04:39:24: antoine commented


Fixed in r9463, this was caused by the av-sync stuff #835 in r9372.

When not av-syncing (ie with older clients), we would call make_data_packet_cb from the UI thread instead of the encode thread...

@totaam totaam closed this as completed May 21, 2015
@totaam totaam added the v0.15.x label Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant